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

Refactored MainActivity and AppDataUsageFragment to Kotlin, introduce… #190

Closed
wants to merge 2 commits into from

Conversation

yatiksihag01
Copy link

@yatiksihag01 yatiksihag01 commented Jul 20, 2023

  • Migrated MainActivity.java and AppDataUsageFragment.java to Kotlin.
  • Implemented single responsibility principle and survive configuration changes by moving backend logic to DataUsageViewModel.kt and UsageDataHelperImpl.kt.
  • Created a new UsageDataAdapter.kt using AsyncListDiffer and moved click listeners into AppDataUsageFragment.kt, which will improve efficiency.
  • Replaced AsyncTask operations with Kotlin coroutines, significantly decreasing AppDataUsageFragment.kt loading time.
  • Added a debug suffix for the debug package to allow the installation of both release and debug versions on the device simultaneously.
  • Minor bug fixes.

…d DataUsageViewModel and UsageDataHelperImpl for single responsibility, optimized UsageDataAdapter with AsyncListDiffer and click listeners in AppDataUsageFragment, replaced AsyncTask with Kotlin coroutines, and added debug suffix for debug package.
@soshial
Copy link

soshial commented Jul 20, 2023

It used AsyncTask?? Sweet mother of god!
Thank you @yatiksihag01

@yatiksihag01
Copy link
Author

@soshial 97% of the code is still in Java, so the use of AsyncTask is expected. The app has a great UI and good features, but the project needs to be migrated to Kotlin following proper architecture guidelines to reduce complexity and improve performance.

@itsdrnoob
Copy link
Owner

@yatiksihag01 So I've been testing out the PR, and here's a few things the I noticed.

  • The data usage count of "System Apps" in the "AppDataUsageFragment" seems to be inaccurate.
  • "System Apps" list is refreshed again when opened for the first time and the value remains unchanged even when the main "App usage" is refreshed.
  • Upon changing the session, data usage values seem to not update for certain items, which could be an issue with the adapter as the values change once the views are recycled.
  • On refreshing the list, the existing list kind of flashes once before the loading shimmer is shown.
  • The usage filter can be changed during a refresh, which could lead to a potential NullPointerException
  • The loading shimmer is shown every time on switching fragments, which is not ideal when the list is not refreshing.

These were my initial impressions before any in-depth testing, so feel free to update me. Also, the build was compared against the release/2.3.2 build.

@yatiksihag01
Copy link
Author

@itsdrnoob Thanks for your feedback. Sorry I'm a bit busy for few days, but I'll review and address the issues as soon as possible.

@itsdrnoob itsdrnoob changed the base branch from release/2.3.2 to dev/2.4.0 August 13, 2023 13:07
@itsdrnoob
Copy link
Owner

@yatiksihag01 No worries. I'll just keep the PR open for the time being.
I've updated the base branch to dev/2.4.0. Please make sure to fetch the latest changes to avoid any merge conflicts.

@yatiksihag01
Copy link
Author

Thanks. Don't worry I'll fetch the latest changes to prevent any potential merge conflicts.

@yatiksihag01
Copy link
Author

Hi @itsdrnoob, I'm closing this PR and opening a new one as I've rebased this branch onto dev/2.4.0

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.

3 participants