-
Notifications
You must be signed in to change notification settings - Fork 65
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
Move to kotlin #12
base: master
Are you sure you want to change the base?
Move to kotlin #12
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.
Well done! LGTM but there are some minor changes.
const val FRENCH = "fr" | ||
const val ARABIC = "ar" | ||
|
||
private var sharedPreferences: SharedPreferences? = 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.
a lateinit
sharedPreferences
should be more appropriate. don't you think?
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.
sharedPreferences can be null if a class calls a method (for example the method setLocaleIfNeeded(@language language: String)) without initiating sharedPreferences via the method init(context: Context?)
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 I see BUT calling init
is mandatory. It's a prerequisite to use LocaleHelper
. As you can see init is called only once here https://github.com/Wiqaytna-app/wiqaytna_android/pull/12/files#diff-72d398151f1a0977873d51d832bb38f2R33
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.
What do you think about my previous comment?!
const val ARABIC = "ar" | ||
|
||
private var sharedPreferences: SharedPreferences? = null | ||
var currentLocale: String? = 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.
non optional currentLocale
here
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.
currentLocale can be null if a class does not call the method init(context: Context?), but it should be private
No description provided.