Code Reviews

Code reviews are undertaken to promote consistency and quality in the code we write. The more we write code in a similar style, using similar tools, the easier our applications will be to maintain.

Reviews are intended to be real-time examinations of code, not demonstrations of the application functionality. There is no need to have the application up and running during the review.

Reviews should take place early enough in the development cycle that there is time to make changes suggested during the review.

Typically we look at code starting at the UI (JSP) and working our way down through Spring MVC controllers, services, DAOs, and domain objects. We also look at unit tests.

Code Review Checklist

This checklist details most of the things we will discuss during the code review.

Sections:

1. General
2. JSP
3. Web (Controllers)
4. Service
5. Data Access
6. Domain
7. Unit Tests
8. Java - General
9. App Config/Layout

1. General

• Have we accounted for concurrent updates?

2. JSP

• Using JQuery for javascript
• Using JqueryUI plugins for widgets
• Third-party Jquery Plugins?
• No old-style javascript embedded in HTML tags (e.g. onClick)

3. Web/Controllers

• @MVC annotations (@Controller, @RequestMapping etc)
• @Autowired annotation for injected dependencies
• No instance variables instantiated via “new” - injected only
• No business logic - only UI-related logic

4. Service

• @Service annotation, @Autowired annotation for injected dependencies
• No instance variables instantiated via “new” - injected only

5. Data Access

• DAOs do not extend Spring’s HibernateDAOSupport
• DAOs extend MIT’s AbstractHibernateDAO
• DAOs do not use Spring’s HibernateTemplate - if AbstractHibernateDAO does not provide necessary function, use Hibernate’s Session API

6. Domain

• Don’t set attributes inside a getter

7. Java - General

• Use constants declared as static variables rather than hard-coding values in the Java code.
• Place instance variables at the top of the class; getters and setters at the bottom of the class.
• Use log level of “debug” unless there’s a specific reason to use info or error.
• Methods and classes documented accurately with javadoc.
• Method has unit test.

8. Unit Tests

• What is the code coverage?