-
Notifications
You must be signed in to change notification settings - Fork 10
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
Remember hydrometer calibration temp. Fixes #55. #56
Conversation
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.
Looking good, thanks :)
I stuck two comments on that I think would be good to clean up before merging.
} | ||
else { | ||
calibTempEditText.setText("20"); | ||
String calibTemp=preferences.getString(Constants.PREF_HYDROMETER_CALIBRATION_TEMP,"68"); |
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 one should be "20" instead of "68" since it's in Celsius.
@@ -235,7 +235,9 @@ public void onDrawerOpened(View drawerView) { | |||
fragmentList.add(new EditMashProfilesFragment()); | |||
fragmentList.add(new ViewStylesFragment()); | |||
fragmentList.add(new StrikeWaterCalculatorFragment()); | |||
fragmentList.add(new HydrometerTempCalculatorFragment()); | |||
HydrometerTempCalculatorFragment hydrometerTempCalculatorFragment = new HydrometerTempCalculatorFragment(); |
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 that instead of passing in the preferences here, you could do something like this inside of the fragment itself whenever you need to access the preferences:
this.getActivity().getSharedPreferences(Constants.PREFERENCES, Context.MODE_PRIVATE)
My concern being that, if the fragment is destroyed / re-created by Android at some point, the way it is now it might lose the passed in preferences object and hydrometerTempCalculatorFragment.preferences
could end up a null pointer.
I have updated the files as requested - you were right on both counts. |
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.
OK - This looks really good to me :)
Giving the Hydrometer calculator fragment access to global prefs could probably be done more elegantly, but this is simple and works.