- Organizational Standards and Conventions
- Putting the "Engineering" in Software Engineering
- Working with Logging and Tracing
- Project and Packaging File Structure
- Unit Testing Requirements
- Code Completion and Review Process Requirements
- Communicating the Vision the Wiki Way
- Conclusion
- Links to developerWorks Articles
- Reference
This chapter is from the book
This chapter is from the book
This chapter is from the book
Code Completion and Review Process Requirements
The best way to ensure that quality components are being created is to perform a peer review on the code. This is also a time and place for your manager or team lead to understand the work and be confident that this code is ready to deploy. The steps involved in declaring code complete is brief: Before a developer declares code complete, usually he or she checks the code in and tells the project manager or team lead that the code is finished. The PM then checks that component as complete on his or her list and the developer moves on to another function.
What is interesting to me is that we wonder why we have problems. Really, in what other job can we get away with declaring the job finished without someone checking our work or questioning how we might have performed some actions? This chapter provides some guidelines for setting up standards and rules for development, but more important is checking that these standards are being followed.
I personally am not a big fan of checklists. You may not believe me because of my focus on standards, but documentation for documentation’s sake is never fun or productive. Rather than checklists I suggest guidelines. These are things that a developer can do a follow-up check with to ensure that they won’t get flagged during a code review. Performing a code review, by the way, is the first thing the PM or team lead should ask to do on the completed code. Here is a list of potential guidelines:
- The code is really complete. No stubs, No partial have-to-do’s like removing System.out lines or adding comments.
- Some initial static analysis has been done, obvious things are taken care of such as hard-coded properties, and exceptions are handled appropriately.
- Third-party and open source libraries have been disclosed and approved for use within your application.
- The code has been compiled, unit tested, and deployed to a test server for some basic and documented functional tests, depending upon your team approach.
Your team should feel free to come up with a more robust set of completion requirements that outline in detail the tasks that should be accomplished; however, the actual code review process will flag most of those items. Having a documented list of requirements could potentially save time during the review process if the code that is presented has a lot of problems. You will have to weigh the effort involved in enforcing a set of requirements against the skill-level of your development team.
Code Reviews
Code review is a dirty word for many of today’s development teams. I’m sure it has to do with the word review. Being reviewed and having others point out your shortcomings is never a pleasant experience. We should change this name to something that is more acceptable to people. Or maybe we should just get over it and embrace the idea that reviews are about the quality of the product, not the quality of the developer.
Many engineering roles have different levels of experience, usually an apprentice and a journeyman or licensed status. The same is true of software engineers: Some developers are more experienced or more productive than others. Code reviews help with the process of becoming a better developer, not only in the standards category, but through discussion about different approaches within the code. Developers should look at the code review process as a learning process, not a judgment on his or her ability. It is a chance for more experienced members of the team to pass on their experience or suggestions and a chance for everyone to learn from the group experience and build their skills.
Ideally code reviews are scheduled throughout the development cycle. Early code reviews should be scheduled after a developer has a working model of the code, and then, of course, a final review should be scheduled before the code is signed off on. This initial early feedback enables good communication between team members and enables the team lead or architect to understand how architectural decisions are being implemented. It also enables some feedback for previously made architectural decisions as well as clarifications if something is not well understood.
I often leave it to the developers to schedule code reviews during the development cycle. I will suggest that they should schedule a review in the next week or so to make sure their approach is fully understood. PMs can shoulder this burden to ensure it gets built into the project plan. The one deadly sin is blowing off the reviews because of lack of time. Any time spent now will multiply the time saved later. This concept has been proven again and again and so should not be a topic for debate. Developers are notorious for putting off code reviews as long as possible, asking for a little more time to just finish this one section.
You Don’t Have to Know Java for a Code Review
Okay, so the preceding statement is only slightly true—but you don’t have to be an expert. Undoubtedly, you have Java experts on your team who can provide the expertise to uncover any problems. However, everyone learns a little something at group code reviews. Embarrassment should never be the focus of a review for anyone; that is, if you ever expect to have more than one review for that developer.
The idea of a code review is to let the specific developer walk you through the code process step by step. Start with gaining an understanding of what he is trying to accomplish, then look at code files to understand how he has implemented this functionality. You might make several passes through the application, first for general understanding and then to review more mundane items like adherence to standards, handling of exceptions, logging, and so on.
One item to determine is who should actually attend the code reviews. My approach is that this is part of the technical team’s development process, not a step-out review. Management should not generally attend unless they are involved at that level. Having them there only puts pressure on the person who is being reviewed and side-tracks the conversation to other topics. PMs might attend for some of the review but not generally be involved in the technical part of the discussion. Business analysts might also be involved in some part of the review to ensure that the initial requirements were understood and followed within the application.