Skip to content
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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

qandeelabbassi
Copy link

Created a singleton object ExcludedMediaSingleton which provides getter and setter methods for excludedImages and removed the excludedImages list from ImagePickerConfig to avoid TransactionTooLargeException

if (excludedImages != null) {
dest.writeList(this.excludedImages);
}
dest.writeByte((byte) ((ExcludedMediaSingleton.getInstance().getExcludedImages() == null ||
Copy link
Owner

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;
Copy link
Owner

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) {
Copy link
Owner

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.
Copy link
Owner

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 {
Copy link
Owner

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();
Copy link
Owner

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()

@esafirm esafirm closed this Oct 2, 2018
@esafirm esafirm reopened this Oct 2, 2018
@qandeelabbassi
Copy link
Author

I will review your requested changes. I will let you know when it's done and sorry about the signature; that was not intentional.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants