trustify-app.md

Trustify Android App

Code Smell :

[master] : There are several major issues with this branch :

• Lack of structure (see activities or fragments folders for example).
• There is very little (if not at all) description on each classes and comments on long procedures, makes it hard to read and maintain.
• The naming convention is little bit weird, CamelCase and snake_case is mixed on string variables (xml), and there is also a typo on class name.
• Too much dead code and dead function scattered all over the place

[refactoring] : (latest branch) :

• Better structure. Each activities or fragment are grouped to its own resource directory.
• There is still no decent description and code-comments, but by utilizing several libraries like retrofit and dagger, which allow annotation for dependency injection and view binding, make the overall code more simpler and easy to understand.
• The naming convention is better, there is no typo on classes name anymore. Minor issue still remain on string variables (CamelCase and snake_case is mixed, which looks like a legacy issue from master branch)
• Less dead code.

Dependency :

[master] : Not fully utilize graddle (eg : stripe and analytics imported as project), make it hard to track and maintain latest patch vendor. Its looks like there is very little (if not at all) concern about these external code licenses.

[refactoring] : Fully utilize graddle, still having some packages that included as project but its more maintainable. Specifically to technical decision about choosing external dependencies is also logical. The additional retrolamda, which adds several Java 8 syntatic sugar, is also nifty : allowing developer to use, lets say, try with resource, on android runtime. We still need to concern about license stuff for all of these external codes, and provides viewable license footprint somewhere in our app.

Memory management and runtime :

[master] : There are many unhandled exception here and there, and the memory soared to more than 20-35 MB on a view that really looks simple. This could indicate bad practice on memory management manner, like running long-time task on ui thread, inefficient code, too complex layout, or improper drawables generated for different resolution (require more investigation to determine precisely).

[refactoring] : The usage of retrofit (which already async) and layout management seems better, and only use 5-8 MB memory on the exist views, which is pretty good. But i havent see rigorious management on each activities to , lets say, unset all binded view variables/services and execute the gc explicitely onStop() or onDestroy(), which could improve the memory usage furthermore. GC on android is very dumb, all developer should aware of this.

Revision Management :

I see so many branches on repo, and unless they're all still on active sprint, its not a good sign. A branch should only exists on some sprint, and should be deleted once its get merged to master branch or its become irrelevant. There is also no release management whatsoever. No automated build/deployment.

all comment you mentioned in master branch already implemented in the "refactoring" except for adding more comment/description in code.

Hi @toopay!

I'm Loi. Curently I'm working with @sonvp to improve and refactor code base. Thanks for your feedback.

Dependency

[refactoring]
Some libraries don't support gradle therefore I still include those into project.

Memory management and runtime

[refactoring]
But i havent see rigorious management on each activities to , lets say, unset all binded view variables/services and execute the gc explicitely onStop() or onDestroy(), which could improve the memory usage furthermore. GC on android is very dumb, all developer should aware of this.

Please check out BaseFragment and BaseActivity class you can see:
ButterKnife.unbind(this);

This method will unset all binded views.