Code Review on Android Projects - Part I

The Code Review is a tedious process sometimes, but I believe that we need to spend more time on that. Maybe it is an opportunity for you to learn or share some knowledge.

I listed some points that I consider necessary to check in a code review process in Android projects.

Memory Leaks

Imagine you are using a beautiful app. You are using the app for thirty minutes. You've changed the screens a lot of times. When you notice the app is staying slow. It can be memory leaks.

How can I check it on a code review?

• RXcalls that you are implementing the subscribe method, they need to be disposed at "onCleared" state if you are using them on a ViewModel;
• Resources such as AsyncTask, CountDownTimer, Toast, and others need to be cleaned at onDestroyView if you are using on Fragments or onDestroy if you are using on Activities.

Deep layouts

Sometimes we have a ViewGroup that has another ViewGroup as a child. A good suggestion is always to use as ViewGroup ConstraintLayouts to keep the layouts flatter. It can avoid some performance issues in the future.

Layout file names

You need to describe what kind of view element your layout is defined. The name is an important decision when you are creating some layout file.

• Activities -> R.layout.activity_something.xml
• Fragments -> R.layout.fragment_something.xml
• ViewHolder -> R.layout.list_item.xml or R.layout.row_item.xml
• Layout components -> R.layout.component_something.xml

Architecture Violation

Whatever architecture has a flow, the image below shows the dependency rule concept.

The first element is the UI layer, where are some Activities, Fragments, and Views. The UI layer depends on the ViewModels, which provide some data classes that manage the content showed for our UI. We also have some UseCases classes, where some objects provide actions.

For example, these actions can be login, sign-in, send some form to the server, and others. The UseCases layer has dependencies with the ViewModels, but the UseCases layer doesn't know any element from the ViewModels. Besides that, the Entities layer defines some abstractions of data used for the UseCases. If we have an UseCase that provides a login method, we probably have an entity called User. 

As you can see in the picture, there is a dependency flow. The UI layer depends on the ViewModels, it depends on the UseCases. The UseCases depends on the Entities. If there is a UI method that calls for some element from Usecase, something is wrong. It is an architecture violation.

Accessibility

I always recommend a brief text to describe the content of the image displayed for an ImageView. You can do it at contentDescription property.

Sometimes the user has a low vision problem to solve that Android provides a setting to increase the text font size of all apps. If you are using "dp" to text size, your app probably doesn't support this feature. You need to use "sp" for that.

Dead Code

We need to remove:

• Unused imports;
• Unused resources like drawables, strings, colors, and so on;
• Commented code (I know your pain, don't be afraid, remove that please!).