Code Crit
There are four elements to the Code Crit methodology.
Rules
Pre-Review
Code Critique
Constructive Criticism
Rules
Before we even start the process we have to institute rules. By instituting rules we avoid chaos. These rules are the same as the ones followed in an art class while preforming art critiques. Organizations can add to these rules, but should avoid removing the base set.
Respect
- - This encompasses not just a level of respect for your coworkers, but for yourself, your company, your project, the community etc.
- - This also means respect the process. This is not the time to express personal issues you have with coworkers.
- - This is about the code. Period.
Be prepared
- - The programmer and the code should be ready for critique.
- - It is disrespectful to enter a code crit when the programmer and/or the code are not ready for critique.
Neutral Language
- - One must stay neutral where being neutral is required. Speaking in neutral terms keeps the dialog open. Nothing kills a conversation faster than negativity. Perceived or other wise.
Constructive Criticism
- - This is by far one of the more difficult aspects of preforming code reviews. One must learn to not only give constructive criticism but to also recieve it. See below for more information.
Stay Focused
- - The purpose of the code critique is to ensure the code being released works. You have to be focused to read and code. Distractions will lead to mistakes getting through.
Mandatory Participation
- - This was one of the more important rules pertaining to art critiques. Mandatory participation ensures the code is reviewed, that it is understood, and there is another level of responsibility for the release. One set of eyes that have been looking at the code are more likely to miss something.
Pre Review
- Identify your work.
- Run ALL relevant testing.
- Know your code!
- Check that the code meets all requirements.
Do not waste the time of others with unprepared code. The person submitting code to be reviewed has a responsibility. If they don’t complete their part of this process, the rest isn’t going to work. Think of the Pre Code Review as a set of good habits you should cultivate.
Bottom line: Code needs to be ready to be reviewed.
Code Critique
Description
The Description stage is where the work is identified, and an inventory is made using neutral terms. (We are not fixing problems at this point, just adding them to the inventory, described below.)
By compliling an inventory we are proving that we have read the code. Without the inventory code can be pushed through with just a thumbs up and no code has been read. Unread code is unreviewed code. That's dangerous code.
Staying neutral in the description stage is vital. If the programmer submitting the code has followed the rules, and their pre code review steps, it will mean the code is working.
If the code doesn’t work, we shouldn’t be reviewing it.
When reviewing code an expert should be involved. A person shouldn’t be the sole reviewer if they are not experienced with the language. Example: A programmer who only knows C, should not be the sole reviewer for a Python project. The more you know about the language, the better you will be able to critique the code written in it.
What is an Inventory?
An Inventory is observations of the code. We make it for it's main purpose, which is to make us read the code. We can take this proof and use it in our feedback. We should always give some sort of feedback. Code is going to be one of three things. Good, Great, or bad.
Analysis
The Analysis stage is where we will go beyond the inventory we have taken in the Description. We start to talk about the relationships of the code. How it works on it’s own, and how it will work with other code.
A complete examination can now be made of the code. Using the inventory, observations can be made:
Does the code function well?
Will the code work with existing code?
The result of the examination will be a through understanding of the code, which means no surprises.
Interpretation
Description + Analysis = Interpretation
Interpretation stage is where we ask what our earlier observations mean?
Suggestions can be made, in a constructive manner, as to any changes that can or should be made to the code.
This stage is where an opportunity arises that is often missed. It is a chance to make ourselves better developers, better leaders, and use mentorship to benefit the company and community.
Good code should be replied to with "code treats", little bits of code that pertain to what they have done. It should not be given in a negative way.
Great code should be replied to with sincere praise. Things like, "I really like what you did here." and point back to the code. Elaborate a bit more than saying this is great. Give examples.
Bad code should be sent back to the developer with notes on what is wrong, not just "this sucks". Point to what is wrong, and why.
If a person walks away from a code review feeling emotionally and verbally beaten they lose their love of programming. Disgruntled, low morale employees are not going to produce quality code. They are going to be looking for other jobs.
Judgement
Can the code ship?
Yes?
Ship it.
No?
Fix it.
Fixes will need to be reviewed. It won’t take as long as we are just reviewing the fix. This next pass can be treated as a second set of eyes as long as we have reached a consensus from the previous review.
Constructive Criticism
In theory constructive criticism is an easy concept to grasp. Putting it into practice is much more difficult.
Constructive criticism is an art form in and of itself. One has to master not only being able to give, but being able to recieve. As a recipiant, one will often have to be able to listen and understand they will not always like what they hear.
Information needs to be communicated clearly and without the use of compliment sandwiches. The goal is to actually give valuable feedback that will contribute to improved code and knowledgable programmers.
Building enjoyable and open lines of communication. Together we should want to help others grow and improve.
Communication and constructive criticism are what makes the whole process work.