Refactored MainActivity and AppDataUsageFragment to Kotlin, introduce…

itsdrnoob / DataMonitor

Data usage monitoring app for android.

GNU General Public License v3.0

532 stars 50 forks source link

Refactored MainActivity and AppDataUsageFragment to Kotlin, introduce… #190

Closed yatiksihag01 closed 10 months ago

yatiksihag01 commented 1 year ago

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.

soshial commented 1 year ago

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

yatiksihag01 commented 1 year ago

@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 commented 11 months ago

@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 commented 11 months ago

@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 commented 11 months ago

@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 commented 11 months ago

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

yatiksihag01 commented 10 months ago

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