Code reviews and “sensitive egos”

From the topic, you can already tell that this is probably going to be bothering some people. It’s okay, we will get through it together.

Photo by Alvaro Reyes on Unsplash

We’ll be talking about the impact of code reviews on the day-to-day life of a software engineer.

Code reviews explained

Imagine there’s a software team — A-Team. Stella is on A-Team, and there’s new functionality to be added to an application, or some bug fixes that need to be done, or even some refactoring to make improvements on the existing codebase. Now Stella is proudly taking on this task. She probably has Ayo and Alex as her teammates. And to have this done, she needs to write some code, and this code needs to be checked by her teammates before getting added to the codebase. The check is important because of course, she doesn’t want to add something that’ll break the application.

Now, the developer — Stella in this case has to create something called a pull request (PR) or a merge request (MR). This is usually a request to team members (Alex and Ayo in this case) to review the new code and suggest improvements where necessary.

There are different forms of code reviews. They don’t always have to be in form of PRs or MRs. Code reviews can also be done while pair programming.

Code reviews are important because they help with knowledge sharing. If Stella was a new member of the team, taking on a task like that will help her navigate through the codebase, give her an idea of what the code does, take her through the nooks and crannies of the codebase, try to figure out stuff herself, and basically get onboarded gradually into the team.

Reading code vs Writing code

When doing code reviews (reading code), it’s also a good way to assess the readability of your code because lots of times, our mindsets and thought processes when writing code is way different from when reading it.

When writing code, you most likely have a particular task you want to handle, so you might be taking care to ensure that your code is clean. However, your topmost priority is to ensure that your code works and that it implements what is intended.

When you’re reading code, on the other hand, it’s a bit different because you are checking for mistakes, potential bugs, errors, error handling, things that could be improved, if the code introduces defects etc. In essence, your “criticism shades” are pretty much on, not for the purpose of being bossy or to make everyone unhappy, but to ensure quality code is being delivered.

Before you approve PRs/MRs

Before approving a pull request or a merge request, you might want to check these:

• Feature matches specification — E.g if Stella has been asked to fix a bug preventing users from logging in, after introducing new code, that feature should now work. A user should be able to log in successfully without any error. So you should check for the functionality, and that it performs well.
• Edge cases — You want to check for edge cases, exceptions, cases that are not clear, unobvious cases etc. This implies you need to do a little bit of thinking. As a user, what are the possible things I’m likely to do that might not be obvious at first glance?
• Security vulnerabilities
• Blindly approving code — It is better to avoid approving code than blindly approving, that is, without a proper review as this may be really detrimental. However, I also know that there’s a tendency for people who are new to programming or to a codebase to avoid reviewing code because they don’t want to ask questions that may not seem smart, or because they don’t just want to embarrass themselves. I think it’s very important you take a jab at doing reviews no matter how scary it seems. This helps you improve the knowledge you already have, and see the different ways to write code, and also learn different algorithms, code styles etc.

Code review etiquettes

I have a couple of tips on commenting during code reviews. I guess this is where the “sensitive egos” part of the topic came from.

Photo by Randalyn Hill on Unsplash

• Mannerism

You want to encourage the code author to make corrections, as opposed to making demands. You want to use phrases like these:
“Do you think…”
“Could you explain why you did this?”
“It looks like this was supposed to be this way”
“You may have forgotten to put this instead of this”

As much as possible, try to avoid phrases like these:
“This is terrible”
“This looks stupid”
“What were you thinking?”
“Change this now!”

Note that these comments are not always meant for scolding people. You could give a thumbs up to a team member if there’s nothing to criticise. Also, it’s useful to provide useful links to the code author when explaining concepts such as code styles or style guides.

• Use examples

Another thing I found really helpful is using examples. If someone did not do something in a relatively proper way, you could use an example to explain the point you’re trying to make as opposed to just using words, because you might not be so clear if you only use code scripts.

Successful code review tips

As an author or code reviewer, there are some useful tips on code review:

• Read the code in full

As a code reviewer, navigate your way around the code, understand what the code is supposed to do, make some comments without initially publishing. I have found that sometimes when you make some comments, then you go down into the code and see that the comments you made were already addressed somewhere in the code. When you’re done and the comment is still relevant, you can then go ahead and publish it.

• Keep it short

If you’re a code author like Stella, and you’re creating your PR/MR, you want to keep it as small as possible. The reality is that people have different attention spans, and it is generally easier for people to read short things or things that are not cumbersome, and still be able to get enough knowledge from them, as opposed to reading long lines of code. Therefore, keep your PRs/MRs small, so anyone reading can understand and give you good feedback.

I also understand that this is not always possible as you might be trying to make a big change, and it might be difficult to break it down. In such cases, you can have those big changes, but I don’t think it should be a norm, having many changes in your PRs.

• Clear communication

It’s very important that you ensure that you communicate clearly. If you are the one giving feedback, particularly as a reviewer, and you’re trying to get the code author to understand something, ensure you explain using examples and links so it’s clear, and to avoid back and forths. As much as possible, try to be clear when giving feedback on the code written.

• Focus on the code

Also, it’s important to bear in mind that you are reviewing the code and not the author. So focus your attention on making the code better, and not attacking the code author by saying things like “You did this again,” or “I told you about this before.” Focus on the code, and making it better.

• De-escalate early

Ensure you de-escalate as soon as possible. When there are too many comments being exchanged on a code review, you can decide as a code author to have a one-on-one discussion with the code reviewer to talk through your thought process, so that you don’t waste time or blow things out of proportion.

Those are some things to note as a code reviewer, and as a code author. I hope you learnt a thing or two.