Android Code Review Checklist

Basic Code Review Checklist

Let’s discuss about the basic code review checklist, which can be very handy if you are a beginner in code reviews and/or during initial code reviews.

While reviewing the code, ask yourself the following basic questions:

1. Am I able to understand the code easily?
2. Is the code written following the coding standards/guidelines?
3. Is the same code duplicated more than twice?
4. Can I unit test / debug the code easily to find the root cause?
5. Is this function or class too big? If yes, is the function or class having too many responsibilities?

If you feel that the answer is not satisfactory to any of the above questions, then you can suggest/recommend code changes.

Detailed Code Review Checklist

The following code review checklist gives an idea about the various aspects you need to consider while reviewing the code:

1. Code formatting

While going through the code, check the code formatting to improve readability and ensure that there are no blockers:

a) Use alignments (left margin), proper white space. Also ensure that code block starting point and ending point are easily identifiable.

b) Ensure that proper naming conventions (Pascal, CamelCase etc.) have been followed. 

c) Code should fit in the standard 14 inch laptop screen.  There shouldn’t be a need to scroll horizontally to view the code. In a 21 inch monitor, other windows (toolbox, properties etc.) can be opened while modifying code, so always write code keeping in view a 14 inch monitor.

d) Remove the commented code as this is always a blocker, while going through the code. Commented code can be obtained from Source Control (like SVN), if required.

2. Architecture

a) The code should follow the defined architecture.

1. Separation of Concerns followed
   • Split into multiple layers and tiers as per requirements (Presentation, Business and Data layers).
   • Split into respective files (HTML, JavaScript and CSS).

2. Code is in sync with existing code patterns/technologies.
3. Design patterns: Use appropriate design pattern (if it helps), after completely understanding the problem and context.

3. Coding best practices

1. No hard coding, use constants/configuration values.
2. Group similar values under an enumeration (enum).
3. Comments – Do not write comments for what you are doing, instead write comments on why you are doing. Specify about any hacks, workaround and temporary fixes. Additionally, mention pending tasks in your to-do comments, which can be tracked easily.
4. Avoid multiple if/else blocks.
5. Use framework features, wherever possible instead of writing custom code.

4. Non Functional requirements

a) Maintainability (Supportability) – The application should require the least amount of effort to support in near future. It should be easy to identify and fix a defect.

1. Readability: Code should be self-explanatory. Get a feel of story reading, while going through the code. Use appropriate name for variables, functions and classes. If you are taking more time to understand the code, then either code needs refactoring or at least comments have to be written to make it clear.
2. Testability: The code should be easy to test. Refactor into a separate function (if required). Use interfaces while talking to other layers, as interfaces can be mocked easily. Try to avoid static functions, singleton classes as these are not easily testable by mocks.
3. Debuggability: Provide support to log the flow of control, parameter data and exception details to find the root cause easily. If you are using Log4Net like component then add support for database logging also, as querying the log table is easy.
4. Configurability: Keep the configurable values in place (XML file, database table) so that no code changes are required, if the data is changed frequently.

b) Reusability

1. DRY (Do not Repeat Yourself) principle: The same code should not be repeated more than twice.
2. Consider reusable services, functions and components.
3. Consider generic functions and classes.

c) Reliability – Exception handling and cleanup (dispose) resources.

d) Extensibility – Easy to add enhancements with minimal changes to the existing code. One component should be easily replaceable by a better component.

e) Security – Authentication, authorization, input data validation against security threats such as SQL injections and Cross Site Scripting (XSS), encrypting the sensitive data (passwords, credit card information etc.)

f) Performance

1. Use a data type that best suits the needs such as StringBuilder, generic collection classes.
2. Lazy loading, asynchronous and parallel processing.
3. Caching and session/application data.

g) Scalability – Consider if it supports a large user base/data? Can this be deployed into web farms?

h) Usability – Put yourself in the shoes of a end-user and ascertain, if the user interface/API is easy to understand and use. If you are not convinced with the user interface design, then start discussing your ideas with the business analyst.

5. Object-Oriented Analysis and Design (OOAD) Principles

1. Single Responsibility Principle (SRS): Do not place more than one responsibility into a single class or function, refactor into separate classes and functions.
2. Open Closed Principle: While adding new functionality, existing code should not be modified. New functionality should be written in new classes and functions.
3. Liskov substitutability principle: The child class should not change the behavior (meaning) of the parent class. The child class can be used as a substitute for a base class.
4. Interface segregation: Do not create lengthy interfaces, instead split them into smaller interfaces based on the functionality. The interface should not contain any dependencies (parameters), which are not required for the expected functionality.
5. Dependency Injection: Do not hardcode the dependencies, instead inject them.

In most cases the principles are interrelated, following one principle automatically satisfies other principles. For e.g: if the ‘Single Responsibility Principle’ is followed, then Reusability and Testability will automatically increase.

In a few cases, one requirement may contradict with other requirement. So need to trade-off based on the importance of the weight-age, e.g. Performance vs Security. Too many checks and logging at multiple layers (UI, Middle tier, Database) would decrease the performance of an application. But few applications, especially relating to finance and banking require multiple checks, audit logging etc. So it is ok to compromise a little on performance to provide enhanced security.

Tools for Code Reviews

1. The first step while assessing the code quality of the entire project is through a static code analysis tool. Use the tools (based on technology) such as SonarQube, NDepend, FxCop, TFS code analysis rules. There is a myth that static code analysis tools are only for managers.
2. Use plug-ins such as Resharper, which suggests the best practices in Visual studio.
3. To track the code review comments use the tools like Crucible, Bitbucket and TFS code review process.

Android Java Code Review Checklist

1. Is the functionality correct?
2. Are the classes named suitably?
3. Are the functions named suitably?
4. How's the datastructure being used? Is it the correct DS or it needs improvement?
5. Can the classes be further borken into small classes?
6. Do we need an interface?
7. If it contain functions that can be reused later then are there Utils created for them?
8. Can it use already available Util functions?
9. Does the large set of input for a function deserve a new bean to be created?
10. Can we decrease the coupling?
11. Are all the input arguments being validated?
12. How will the functionalities affect the performance of the app - time and memory?
13. Is it absolutely necessary to run it on UI therad or would a background thread suffice?
14. Are all the fail points handled?
15. Does it degrade gracefully in case of unknown failures?
16. Are the app-doing level standards being followed?
17. Is the correct form of storage being used and is memory available for storage?
• Shared Preference
• DB
• SD Card
• Internal Storage
18. Are the operations thread safe?
19. What pieces of the component can be executed in parallel?
20. Are the layouts suitable for all the screen dimensions?
21. What classes would become obsolete after this implementation?