Java Code Review Techniques

Code review is a multi steps process that does systematic examination of computer source code. The purpose of this process is to find-fix the short fall of source code and improving the quality of software. The process comes in existing after completing of coding phase in SDLC. But this is not a fix rule it may comes at any time after completing the system. The purpose of code review is –

1.      Make system better.

2.      Improve developer skill.

3.      Easy maintenance.

4.      Push system to onwards perfection.

5.       Make system error and bug free.

In general term code review is an act that comes after a lot of practice. These days it emerges as a professional domain. And the professionals those are serving for this art is getting a huge amount as his perk.

Code review steps –

1.      Preparation step – In this step determines the inspection/review target, inspector role or roles. Reading technique and reading technique specified also included at this stage.

2.      Inspection step – in this step inspector looks around the source code with multi perspective view and find out the improvement places and corners according to his/her assigned role. He/she may include some tools to find the defect of code. During the looking process he/she also looks for compilation warning.

3.      Rework steps – in inspection step, the work is actually producing output that is input for this step. When inspection steps list out the improvement places and corners actually it prepares the task for rework steps. Rework steps includes –

a.       Changing concepts for better performance and maintenance. If the current approach is fine then it may go with this approach, but need to convince to inspector.

b.      Methodology improvement.

c.       Rework on file arrangement.

d.      Make generic concepts in term of system.          

4.      Follow up – correction verified and reviewer makes sure that new bugs have not been introduced from current change.

Code review roles –

As introduced that code review is an art that comes after a huge practice. On the basis of persons expertise they may assign some roles to complete the code review cycle. The code review roles are –

1.      Readers/presenter – the reader explain the product being inspected/reviewed on behalf of business system analyst. The actual flow of the system.

2.      Organizer – the organizer develops a plan for development and review. Organizer also defines the time schedule for the complete work. Actually organizer a person who is capable to estimate the time according to its task.

3.      Author – is a developer who wrote the code is being reviewed and inspected. In rework phase it is author responsibility to correct the code according to inspector and reviewer. Even, developer thinks the current approach is better than the advised approach he/she may communicate with inspector concerning current and advised approach. Communication must flow with both directions.

4.      Recorder – the person is responsible to document the issue and summarize the defects as well the improvement list. He/she also responsible to elaborate the suggestion, the back end technique why he/she is advising such steps.

5.      Moderator – the moderator is responsible for leading the review team or group and keeping them focused on their goals. Moderator setups the environment and regulate the discussion to keep review focused. They also manage the time regarding assigned inspector role and task. The responsibility of resource management is also under the moderator.

6.      Reviewer/inspector – all members in code review team assumes under this role. The goal to identify the defects and improvement space.

Communication among reviewer team –

Communication should flow in both directions like xylem and phloem. Communication makes the system sound and perfect. So, all team members keep in mind that there should not be any hurdle in the way of communication. But also keep in mind that communication should be relevant, irrelevant communication never be appreciated.

Considerable facts during review process –

1.      High level architecture (Recommendation) –

1.1  Modular application – at current time whatever I got the system is high coupled, and I think this is not right approach for development. If possible then we should try to minimize the coupling and optimize the code as a separate module. For instance there is a login module – we should try to make all login related files inside a login module. There is possibility that some common methods, we shall try to make this method generics and keep in common utility package. The best example of modular application is LINUX operating system. The benefits of this approach is –

a.       Code becomes good from ugly.

b.      Make distribution development.

c.       Give developers more flexibility and maintainability.

d.      This approach provides the approach to manage the complexity.

e.       It increases software quality.

1.2  Upgrade front end framework – at current time it is using struts 1.1 data type definition and 1.0 configuration architecture. Is it possible to update to struts 1.2. If it is possible then we can protect ourselves to carry huge java scripts. We also get some better response time using tiles.

2.      Code review dimensions – at current time code review process will focus on some limited dimension. The dimension will cover two angles those are coding and performance (will cover in next blog under the Heading Java performance tuning). Coding will cover all aspect in such a ratio so that it produces better maintainability.

2.1  Coding: - To write everything about the coding standards are possible here. But some common coding guidelines those are must be followed during the code. The best practice for coding are as follows –

2.1.1        Always use java coding standards.

2.1.2        The standard size of java class file should not larger than 1000 lines while a function size should not larger than 400 lines.

2.1.3        The naming convention should camel case, on everywhere. Everywhere means, starting from Html components to java property declaration.

2.1.4        Don’t write extra white space or return (Enter) space on .java files.

2.1.5        Must implements standard logging concepts inside your code.   

2.1.6        Must write a standard (according to computer screen size) with of code instruction so that it can viewed at one go. Supposed a screen size is 100^th character wide, then code line should be less than the size, and this process will go all nested instructions.

2.1.7        Never use confusing property or function name, it should be specific to it’s intend task.

2.1.8        Try to write generic function instead of writing duplicate function.

2.1.9        Till not necessary, always avoid synchronized function, block, object, declaration or uses. For example to use Array list is better preferable rather than vector.

2.1.10    Using branching is more preferable then non-branching. Even if single line of control must have a scope with opening and closing braces.

2.1.11    To get details about the java coding convention click here – Java Coding Standards.

2.1.12    During the coding also strictly consider the following points –

OBJECTIVES
DEVIATION
R#	Considering points
1.	Does the application follow any design pattern?
2.	Does the code completely follow that/those design pattern?
3.	Is every parameter of any method passing appropriate value or reference?
4.	Does every method return the correct value at every method return point?
5.	If an expression will not fit on a single line then, break it according to the principles in Sun standard.
6.	Does each line contain at most one simple statement?
OMMISSION
7.	Does the code completely implement the design?
8.	Are there any requirements of design that were not implemented?
DEFECT
Class/Interface declaration
9.	Does each class have an appropriate constructor?
10.	Is class name is noun? (it is good to limit the class name max 32 char )
11.	Do any subclasses have common members that should be in the super class?
12.	Does each java source file having one public class or interface?
13.	Does java source have the following ordering: Package and Import statements, beginning comments, Class and Interface declarations?
14.	Can the class inheritance hierarchy be simplified?
Variables and constant declaration
15.	Are descriptive variable and constant names used in accord with naming conventions?
16.	Is every variable correctly typed?
17.	Is every variable properly initialized?
18.	Are all for-loop control variables declared in the loop header?
19.	Are all for-loop moving forward or backward direction?
20.	Are there variables that should be constants?
21.	Are there any unnecessary variable declared inside any scope?
22.	Is instance variables declared publicly?
23.	Is instance variable initialized?
24.	Are arrays should be declared with their brackets next to the type?
25.	Variables should be initialized where they are declared and scope should be smallest. Ensure it?
26.	Are there attributes that should be local variables?
27.	Is variable declaration adherence the java type safe feature?
28.	Do all attributes have appropriate access modifiers (private, protected, public)?
29.	Are there static attributes that should be non-static or vice-versa?
Method declaration/definition
30.	Are descriptive method names used in accord with naming conventions?
31.	Do all methods have appropriate access modifiers (private, protected, public)?
32.	Is every method parameter value checked before being used?
33.	Is exception handled generically inside method?
34.	Is there any open resource?
35.	Is public or protected method document following java doc convention?
36.	Are all scope used object nullify?
37.	Is every method is single task oriented?
38.	Are there static methods that should be non-static or vice-versa?
Data Reference
39.	For every array reference: Is each subscript value within the defined bounds?
40.	For every object or array reference: Is the value certain to be non-null?
Computation/Numeric
41.	Are there any computations with mixed data types?
42.	Is overflow or underflow possible during a computation?
43.	For each expression with more than one operator: Are the assumptions about order of evaluation and precedence correct?
44.	Are parentheses used to avoid ambiguity?
45.	Does the code systematically prevent rounding errors?
46.	Does the code avoid additions and subtractions on numbers with greatly different magnitudes?
47.	Are divisors tested for zero or noise?
Comparison/Relational
48.	Has each Boolean expression been simplified by driving negations inward?
49.	For every Boolean test: Is the correct condition checked?
50.	Are there any comparisons between variables of inconsistent types?
51.	Are the comparison operators correct?
52.	Is each Boolean expression correct?
53.	Are there improper and unnoticed side-effects of a comparison?
54.	Has an "&" inadvertently been interchanged with a "&&" or a "|" for a "||"?
55.	Does the code avoid comparing floating-point numbers for equality?
56.	Is every three-way branch (less, equal, greater) covered?
Control Flow
57.	For each loop: Is the best choice of looping constructs used?
58.	Will all loops terminate?
59.	When there are multiple exits from a loop, is each exit necessary and handled properly?
60.	Does each switch statement have a default case?
61.	Are missing switch case break statements correct and marked with a comment?
62.	Is the nesting of loops and branches too deep, and is it correct?
63.	Can any nested if statements be converted into a switch statement?
64.	Are null bodied control structures correct and marked with braces or comments?
65.	Does every method terminate?
66.	Are all exceptions handled appropriately?
67.	Do named break statements send control to the right place?
Input/output
68.	Have all files been opened before use?
69.	Are the attributes of the open statement consistent with the use of the file?
70.	Have all files been closed after use?
71.	Is buffered data flushed?
72.	Are there spelling or grammatical errors in any text printed or displayed?
73.	Are error conditions checked?
74.	Are files checked for existence before attempting to access them?
75.	Are all I/O exceptions handled in a reasonable way?
Module Interface
76.	Are the number, order, types, and values of parameters in every method call in agreement with the called method's declaration?
77.	Do the values in units agree (e.g., inches versus yards)?
78.	If an object or array is passed, does it get changed, and changed correctly by the called method?
Comment
79.	Does every method, class, and file have an appropriate header comment?
80.	Does every attribute, variable or constant declaration have a comment?
81.	Is the underlying behavior of each method and class expressed in plain language?
82.	Is the header comment for each method and class consistent with the behavior of the method or class?
83.	Are all comments consistent with the code?
84.	Do the comments help in understanding the code?
85.	Are there enough comments in the code?
86.	Are there too many comments in the code?
Layout and Packing
87.	Is a standard indentation and layout format used consistently?
88.	For each method: Is it no more than about 60 lines long?
89.	For each compile module: Is no more than about 600 lines long?
Modularity
90.	Is there a low level of coupling between modules (methods and classes)?
91.	Is there a high level of cohesion within each module (methods or class)?
92.	Is there repetitive code that could be replaced by a call to a method that provides the behavior of the repetitive code?
93.	Are the Java class libraries used where and when appropriate?
Storage Usage
94.	Are arrays large enough?
95.	Are object and array references set to null once the object or array is no longer needed?
Performance
96.	Can better data structures or more efficient algorithms be used?
97.	Are logical tests arranged such that the often successful and inexpensive tests precede the more pensive and less frequently successful tests?
98.	Can the cost of re-computing a value be reduced by computing it once and storing the results?
99.	Is every result that is computed and stored actually used?
100.	Can a computation be moved outside a loop?
101.	Are there tests within a loop that do not need to be done?
102.	Can a short loop be unrolled?
103.	Are there two loops operating on the same data that can be combined into one?
104.	Are frequently used variables declared register?
105.	Are short and commonly called methods declared inline?
106.	Are timeouts or error traps used for external device accesses?
INCONSISTENCY
Performance
107.	Are there any code implement in inconsistent way?
AMBIGUITY
Variable and Constant Declaration
108.	Are there variables with confusingly similar names?
109.	Are all variables properly defined with meaningful, consistent, and clear names?
Performance
110.	Are any modules excessively complex and should be restructured or split into multiple routines?
REDUNDANCY
Variables
111.	Are there any redundant or unused variables or attributes?
112.	Could any non-local variables be made local?
Method Definition
113.	Are there any uncalled or unneeded methods?
Performance
114.	Can any code be replaced by calls to external reusable objects?
115.	Are there any blocks of repeated code that could be condensed into a single method?
116.	Are there any leftover stubs or test routines in the code?
SIDE-EFFECT
Method Definition
117.	After changing of prototype of method, Have class which calls it considered yet?
Data Base
118.	Do Upgrading and Migration process follow up changing of structures or contents of a project’s data base?