[Image_Picker] Use cache instance on Android #1877
Conversation
cyanglaz
left a comment
cyanglaz
left a comment
There was a problem hiding this comment.
Thank you for the contribution! I left some comments.
| private static final String SHARED_PREFERENCES_NAME = "flutter_image_picker_shared_preference"; | ||
|
|
||
| private static SharedPreferences getFilePref; | ||
| private SharedPreferences prefs; |
There was a problem hiding this comment.
I see all the null check are removed. I am not a Java expert but how do we ensure the prefs is not null to prevent NPE?
There was a problem hiding this comment.
.static methods and variables are associated with the class they are declared.
Non static methods and variables are associated with an instance of an object of a given type.
I removed static declaration for prefs to make it part of an instance of ImagePickerCache. To access prefs, I need an instance of ImagePickerCache.
So we know prefs is never null because it is being instantiated in the constructor of ImagePickerCache.
As prefs is no longer static, the static methods were converted to instance methods as well. Static methods can't access non static variables/methods after all.
As a rule of thumb: Avoid static methods/variables. They are hard to write unit tests and they are memory leak and error prone.
| if (getFilePref == null) { | ||
| return; | ||
| } | ||
| Map<String, Double> getMaxDimensions() { |
There was a problem hiding this comment.
where is this method used?
There was a problem hiding this comment.
I forgot to remove it from my first PR. Removed.
| MAP_KEY_MAX_WIDTH, | ||
| Double.longBitsToDouble(getFilePref.getLong(SHARED_PREFERENCE_MAX_WIDTH_KEY, 0))); | ||
| } | ||
| final long maxWidthValue = prefs.getLong(SHARED_PREFERENCE_MAX_WIDTH_KEY, 0); |
There was a problem hiding this comment.
Why did we remove the contains check? Is it not necessary for long values?
There was a problem hiding this comment.
I forgot to remove it from my first PR. reverted.
|
@cyanglaz mind to take another look? |
cyanglaz
added
submit queue
3 participants
Description
This PR changes how the access to the Android plugin cache is done.
All static methods were converted to instance methods and their references updated.
This change may help with testing in the future.
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze) does not report any problems on my PR.Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?