-
-
Notifications
You must be signed in to change notification settings - Fork 216
Convert MainActivity to Kotlin #2830
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
base: main
Are you sure you want to change the base?
Conversation
TheLastProject
left a comment
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.
Seems like a decent start, but this can definitely be more Kotlin-y :)
| private var binding: MainActivityBinding? = null | ||
| private var contentMainBinding: ContentMainBinding? = null | ||
| private var mDatabase: SQLiteDatabase? = null | ||
| private var mAdapter: LoyaltyCardCursorAdapter? = null | ||
| private var mCurrentActionMode: ActionMode? = null | ||
| private var mSearchView: SearchView? = null | ||
| private var mLoyaltyCardCount = 0 | ||
| @JvmField | ||
| var mFilter: String = "" | ||
| private var currentQuery = "" | ||
| private var finalQuery = "" | ||
| protected var mGroup: Any? = null | ||
| protected var mOrder: LoyaltyCardOrder? = LoyaltyCardOrder.Alpha | ||
| protected var mOrderDirection: LoyaltyCardOrderDirection? = LoyaltyCardOrderDirection.Ascending | ||
| protected var selectedTab: Int = 0 | ||
| private var mCardList: RecyclerView? = null | ||
| private var mHelpSection: View? = null | ||
| private var mNoMatchingCardsText: View? = null | ||
| private var mNoGroupCardsText: View? = null | ||
| private var groupsTabLayout: TabLayout? = null | ||
| private var mUpdateLoyaltyCardListRunnable: Runnable? = null | ||
| private var mBarcodeScannerLauncher: ActivityResultLauncher<Intent?>? = null | ||
| private var mSettingsLauncher: ActivityResultLauncher<Intent?>? = 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.
The vast majority of these probably work better as lateinit, given they're only null because the value isn't known yet. See other activities.
I'm skipping reviewing all the !! calls now, because turning stuff into lateinit will change that flow too most likely.
| builder.setNegativeButton( | ||
| R.string.cancel, | ||
| DialogInterface.OnClickListener { dialog: DialogInterface?, which: Int -> dialog!!.dismiss() }) |
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.
Please be consistent in formatting and put the ending ) on its own line :)
| @Override | ||
| public boolean onActionItemClicked(ActionMode inputMode, MenuItem inputItem) { | ||
| override fun onActionItemClicked(inputMode: ActionMode, inputItem: MenuItem): Boolean { | ||
| if (inputItem.getItemId() == R.id.action_share) { |
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 is a lot of if/else in Java due to Java limitations with these R.id things, but I think in Kotlin you might be able to just use a when switch?
| long twentyFourHoursAgo = System.currentTimeMillis() - (1000 * 60 * 60 * 24); | ||
|
|
||
| File[] tempFiles = getCacheDir().listFiles(); | ||
| Thread(Runnable { |
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 at
| Thread { |
Runnable is needed
| mHelpSection = contentMainBinding!!.helpSection | ||
| mNoMatchingCardsText = contentMainBinding!!.noMatchingCardsText | ||
| mNoGroupCardsText = contentMainBinding!!.noGroupCardsText | ||
| mCardList = contentMainBinding!!.list |
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 we can just get rid of these variables, Kotlin is a lot smarter about types and directly calling the elements in the contentMainBinding will probably be fine in Kotlin and even more readable and save some code complexity.
| updateLoyaltyCardList(true) | ||
|
|
||
| FloatingActionButton addButton = binding.fabAdd; | ||
| // End of active tab logic |
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.
Let's make sure to keep the "end of tab logic" right below the tab logic instead of just above the add button logic (so: no newline before, but a newline after)
| val addButton = binding!!.fabAdd | ||
|
|
||
| addButton.setOnClickListener(v -> { | ||
| Intent intent = new Intent(getApplicationContext(), ScanActivity.class); | ||
| Bundle bundle = new Bundle(); | ||
| addButton.setOnClickListener(View.OnClickListener { v: View? -> |
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.
Let's also just call binding.fabAdd directly here, should work fine in Kotlin :)
| override fun onOptionsItemSelected(inputItem: MenuItem): Boolean { | ||
| val id = inputItem.getItemId() | ||
|
|
||
| if (id == android.R.id.home) { |
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 all these if id = can also just be a when statement in Kotlin :)
No description provided.