Programmers dread the "code review". This is where the programmer sits down with his or her peers and their peers bash their fellow programmer's code - and, in not so many words, tell the programmer that they are a terrible person. The code review is about ego boosting/ego crushing disguised as a quality assurance practice. Well, that's what happens in a lot of code reviews and, when it happens, it is a form of bullying. The word "review" implies that the code is being judged:
Review: "a formal assessment or examination of something with the possibility or intention of instituting change if necessary."
Judged: "form an opinion or conclusion about" or "give a verdict on someone."
Also, the code review involves not only judgment but the peers are the jury and executioner in some sort of twisted intervention:
Intervention: "an occasion on which a person with an addiction or other behavioral problem is confronted by a group of friends or family members in an attempt to persuade them to address the issue."
Code reviews are the equivalent of walking into the principal's office in school and getting flogged by a student. Everyone dreads the idea, so why do them? Code reviews are actually from the 1970's (no joke!), which means we need to seriously question their relevance. They are outdated and have no place in the modern workforce. Instead, we need to be doing the more modern "code overview".
What is a code overview? Basically, it is an overview of the design of any project that exceeds the general Norris Number of the rest of the team so that the team simply knows about the structure of the project - regardless of whether or not they agree with the overall design. An example is a team that typically develops and maintains 2,000 line apps and needs to know about a 20,000 line app that's been developed, then that's the time for a code overview. The goal of a code overview is to raise the Bus Factor - not point out faults in the code or its design. The bus factor on most projects typically hangs around one in smaller teams. That is, if that one person gets hit by a bus and is killed/maimed, the project will become a hopelessly lost cause from the business perspective of getting new features added to it. Simply knowing the structure of a project is sufficient for a reasonably capable programmer to carry on the work. After all, half the battle of updating a project that a programmer has never touched before is figuring out its structure.
How does one conduct a code overview? Well, it is more of a presentation style to disseminate information. When you think "presentation", you probably think PowerPoint with notes made in Word or similar software and that's exactly what a code overview is. One person shows the work that has been done and other people sit in and learn. It's a formal environment designed to guarantee dissemination of important information. In many cases, it only needs to be 10-15 minutes long because it is intended for a similarly technical audience of peers. The key is to remember that it isn't about putting on a show. What files sit where and what those files do or a database schema with the most important tables highlighted are just two topic ideas for a code overview. The presenter may not even necessarily open up those files, look at any code(!), or look at the database data. Just knowing the structure is generally sufficient. Then make the content of the presentation available in a Word document (or equivalent - e.g. Google Doc). The goal is to get more people up to speed just enough to be able to go in and change a project should the need ever arise. Hopefully it won't, but it's nice to cover the bases.
As a side-benefit, code overviews will happen rarely and mostly toward when the project hits the maintenance phase of development. This allows everyone to focus on their work and occasionally learn about other projects, which saves the organization time and money down the road. It also fosters healthy interactions within the team and smart managers see it as an opportunity to leverage the budget to buy food and simultaneously accomplish other important organizational tasks during the meeting.
Code reviews generally breed a negative, unhealthy atmosphere and nearly everyone going into one has the wrong mindset or gets sucked into the wrong mindset during the review and it turns into a form of bullying in the workplace. As long as the code works and does what it is supposed to do, who cares what it looks like? Code overviews, on the other hand, are a harmless but essential part to creating continuity within an organization.
Review: "a formal assessment or examination of something with the possibility or intention of instituting change if necessary."
Judged: "form an opinion or conclusion about" or "give a verdict on someone."
Also, the code review involves not only judgment but the peers are the jury and executioner in some sort of twisted intervention:
Intervention: "an occasion on which a person with an addiction or other behavioral problem is confronted by a group of friends or family members in an attempt to persuade them to address the issue."
Code reviews are the equivalent of walking into the principal's office in school and getting flogged by a student. Everyone dreads the idea, so why do them? Code reviews are actually from the 1970's (no joke!), which means we need to seriously question their relevance. They are outdated and have no place in the modern workforce. Instead, we need to be doing the more modern "code overview".
What is a code overview? Basically, it is an overview of the design of any project that exceeds the general Norris Number of the rest of the team so that the team simply knows about the structure of the project - regardless of whether or not they agree with the overall design. An example is a team that typically develops and maintains 2,000 line apps and needs to know about a 20,000 line app that's been developed, then that's the time for a code overview. The goal of a code overview is to raise the Bus Factor - not point out faults in the code or its design. The bus factor on most projects typically hangs around one in smaller teams. That is, if that one person gets hit by a bus and is killed/maimed, the project will become a hopelessly lost cause from the business perspective of getting new features added to it. Simply knowing the structure of a project is sufficient for a reasonably capable programmer to carry on the work. After all, half the battle of updating a project that a programmer has never touched before is figuring out its structure.
How does one conduct a code overview? Well, it is more of a presentation style to disseminate information. When you think "presentation", you probably think PowerPoint with notes made in Word or similar software and that's exactly what a code overview is. One person shows the work that has been done and other people sit in and learn. It's a formal environment designed to guarantee dissemination of important information. In many cases, it only needs to be 10-15 minutes long because it is intended for a similarly technical audience of peers. The key is to remember that it isn't about putting on a show. What files sit where and what those files do or a database schema with the most important tables highlighted are just two topic ideas for a code overview. The presenter may not even necessarily open up those files, look at any code(!), or look at the database data. Just knowing the structure is generally sufficient. Then make the content of the presentation available in a Word document (or equivalent - e.g. Google Doc). The goal is to get more people up to speed just enough to be able to go in and change a project should the need ever arise. Hopefully it won't, but it's nice to cover the bases.
As a side-benefit, code overviews will happen rarely and mostly toward when the project hits the maintenance phase of development. This allows everyone to focus on their work and occasionally learn about other projects, which saves the organization time and money down the road. It also fosters healthy interactions within the team and smart managers see it as an opportunity to leverage the budget to buy food and simultaneously accomplish other important organizational tasks during the meeting.
Code reviews generally breed a negative, unhealthy atmosphere and nearly everyone going into one has the wrong mindset or gets sucked into the wrong mindset during the review and it turns into a form of bullying in the workplace. As long as the code works and does what it is supposed to do, who cares what it looks like? Code overviews, on the other hand, are a harmless but essential part to creating continuity within an organization.
I just realized I didn't add one caveat to this post. The ONLY exception is security vulnerabilities within the application. However, it doesn't require a room full of people to figure that out. The organization hires someone who knows security vulnerabilities inside and out and can spot such software bugs a mile away and also treats their fellow human beings with dignity and respect. Part of their job responsibilities are to comb through each software project looking for security vulnerabilities and also make sure various systems are running up-to-date software (aka systems security audits). If there is a consistent problem employee, they use gentle nudges to correct the various misunderstandings through additional education/training modules. But instead of just focusing on one person, they convince the organization to require the entire team to take security vulnerability training that just happens to cover the specific area the one problem employee is "guilty" of. Also have the team's manager put "Hunt for security vulnerabilities in your applications" on each dev's official task list after the training completes. That helps the specific dev save face and everyone, including the organization, benefits from the extra training. Beyond that though, there is no valid reason for a formal code review and certainly no reason for it to be done publicly in front of peers. Like art, everyone writes software differently and everyone's a critic - there's simply no legitimate need to combine the two. Exploited security vulnerabilities carry additional penalties that can cost the organization dearly (loss of data control and theft, data loss, decreased customer trust, legal and financial penalties, embarrassing news articles, etc). Outside of security vulnerabilities, how someone styled that section of code or wrote that class or function is completely irrelevant beyond some minor long-term maintenance costs that probably don't even matter (e.g. the application may stop being useful when the business rules change and either go away or be re-written).
ReplyDeleteIf a dev is intentionally doing something stupid like code golfing, that's a different issue and they just need more work to do. Managers should simply pile on the tasks and put deadlines on each task so that person gets their work done and the code golfing will magically stop all on its own. Once again, there's no need to do a code review. And good devs know better than to code golf on company time.
One other thought I had: This post is primarily about applications that have no particular real world consequences, which is something like 95% of the software that is written in the universe I and most devs operate in. People aren't (likely) going to die if a web application breaks or a video isn't processed or whatever, but EULAs exist for most software just in case. If humans and machines will be involved though, then we can obviously expand what a security/software vulnerability means to include bugs that would be life ending ones. That said, I'm pretty sure everyone on a team would understand what the consequences of killing someone with a software bug are and the responsibilities that follow that (legal, financial, moral and ethical, etc). Duh. The problem is when you get a bunch of people in a room to do a code review of a single person's code, the chance someone's going to make an inappropriate comment about the person and who they are skyrockets. Squashing bugs is one thing. Bullying people is a different thing. Squashing bugs can be done at the 'git commit' level without being offensive. Bullying and ego stroking doesn't belong in the workplace. I'm saying the formal "have a meeting to go over a person's code" is what needs to be re-evaluated and transformed. It's outdated and no longer relevant when we have tools to manage the entire process.
ReplyDelete