-
Notifications
You must be signed in to change notification settings - Fork 327
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Wrapped excluded images list into a singleton #168
base: master
Are you sure you want to change the base?
Conversation
if (excludedImages != null) { | ||
dest.writeList(this.excludedImages); | ||
} | ||
dest.writeByte((byte) ((ExcludedMediaSingleton.getInstance().getExcludedImages() == null || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this is not needed anymore.
@@ -198,10 +191,10 @@ protected ImagePickerConfig(Parcel in) { | |||
this.selectedImages = in.createTypedArrayList(Image.CREATOR); | |||
|
|||
boolean isPresent = in.readByte() != 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove isPresent
this.excludedImages = new ArrayList<>(); | ||
in.readList(this.excludedImages, File.class.getClassLoader()); | ||
} | ||
// if (isPresent) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove all this comments
import java.util.ArrayList; | ||
|
||
/** | ||
* Code written by Qandeel Abbassi on 9/20/2018 at 7:03 PM. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, no signatures or this kind of comment in the repo
/** | ||
* Code written by Qandeel Abbassi on 9/20/2018 at 7:03 PM. | ||
*/ | ||
public class ExcludedMediaSingleton { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be generate singleton for transferring large data between others activities/fragment to ImagePicker.
Can you add ImagePickerConfig.selectedImages
to this as well?
The name could be renamed to something more general as well, and please omit "Singleton" from it.
|
||
public void resetExclusions() { | ||
if (excludedImages != null) { | ||
excludedImages.clear(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this could lead to some unexpected behavior since excludedImages
is shared between ImagePicker and others screen on the app.
If you want to do this, please copy the list instead of just assigning it in setExcludedImageFiles()
I will review your requested changes. I will let you know when it's done and sorry about the signature; that was not intentional. |
Created a singleton object
ExcludedMediaSingleton
which provides getter and setter methods forexcludedImages
and removed theexcludedImages
list fromImagePickerConfig
to avoidTransactionTooLargeException