Reviewing Code Tips & Tricks

Michela Federico
Coder Lifestyle
Published in
6 min readApr 7, 2020

This article is meant to collect a series of tips on how to review your code and hopefully, with time, it will also improve your coding skills.

Code Clean-up

Start from the top of your file (the very first line!) and remove all the variables, properties and methods that are not used.

But be careful! Just checking the number of instances sometimes is not enough.

For example, let’s say that at some point you had the need to use a certain flag so you set it in different places and used it in others.

Then you do some refactoring removing all the conditional statements involving this flag.

Suddenly you end up with a series of instances of the flag where it’s value changes, but that value is not actually used anywhere making your flag obsolete.

In this case, removing the flag altogether will not change the behaviour of your application, your code will look much clearer and also, if another developer is to go through your code he won’t waste time trying to understand what that flag does or why it’s there as it’s never used.

Unused functions are also a big heads-up.

We may have created a function at some point just to split a bigger chunk of code but later on,

we’ve refactored this chunk of code and now our smaller method is not used anymore.

So you’ve ended up with a piece of code that is just hanging in there doing nothing and once again our poor fellow colleague will end up wasting time trying to understand why that method is there and checking to see if anywhere else in the whole application there may be something that triggers it.

Don’t forget that when other people get confused about your code, you are not only wasting their time but there is a good chance that you are also creating an unnecessary chain of back-and-forth between people who may contact each other to seek assistance.

Bottom line is that keeping your code clean is very important and can save a lot of time and stress.

To help you with this cleaning task you may want to make sure that your IDE has a feature that recognises unused variables, properties and functions to a certain degree.

Most editors nowadays have such functionality already built-in, but if this is not the case for you, you may want to have a look at adding some plugins to your dev environment.

Functionalities Review

A good way to start a functionality review may be by reading the code documentation.

First make sure that both code and documentation reflect each other.

Then, make sure that functionalities actually belong to where they are defined.

Let’s say that you are reviewing a component (that you may or may not have written) and you realise that you can gain much advantage by splitting it into different components.

Suddenly you are moving pieces of code around but you may not realise that some of the functionalities that you are leaving behind are actually part of the new logical scopes.

Making sure that every component does only exactly what it should is very important.

Your application may actually work at the time of the refactor but later on some other issues may come up and maybe you’re on holiday on a beautiful beach with no way to be contacted, and your poor fellow colleague has to take over.

He will debug your code looking for that specific component which, logically speaking, should deal with that piece of functionality.

Poor fella has no idea that you mistakenly left that piece of code in the original component. You see where I’m going with that?

Semantic Review

Another thing that tends to cause confusion is naming. Naming something is very important, it’s a way of self-documentation.

You should use as few abbreviations as possible and not be scared to use long names, as long as they’re descriptive.

Semantic reviews cover different scenarios, below you can find a description of the most common ones.

Target-specific methods

Methods that you’ve been calling specifically for a target, are being reused for other targets too tending to create redundant code.

e.g.

Let’s say you have implemented a series of methods to update the state of something:

updateToIdleState()
 updateToStartedState()
 updateToCompleteState()

Later on, you realise that these three methods do exactly the same thing so you remove two of them and you pass a state parameter to the remaining one.

Of course you’re in a rush so you don’t realize that your method is still called updateToIdleState, and because copy paste are your best friends you never actually focus on something’s name.

But then it comes to that infamous day where you’re on that beautiful beach and your colleague has to deal with your code and he’ll wonder why when it’s time to update the state to Start you’re calling an updateToIdleState method (and so frustrations builds up…).

Inconsistent scope in names

e.g.

if (portraitLayout) { showMobileAssets(); }

Let’s suppose that we are on a desktop and we’re resizing the window to have the same aspect ratio used for portrait layouts. What is going to happen then?

Are we going to show assets that supposedly are only used on mobile devices or, are we just going to add a device type check to the conditional statement?

If the first one is the correct answer it means that our ‘showMobileAssets’ name is misleading and it may be better renaming it to ‘showPortraitAssets’,

so that all names cover the same scope of action.

Contextualised naming

We have implemented a piece of functionality that creates a background as it follows:

createBackground(config) { var rect = createRect(); rect.size = config.size || page.size; rect.color = config.color || 0x00

Now let’s say that someone else picks up your code to implement an enhancement which requires code to render a green rectangle.

He/she quickly looks at the names of your methods to see if there’s something that could help out, but just by looking at the methods names, it doesn’t look like there’s a utility method to create rectangles so our colleague ends up creating another method that does exactly the same thing as your createBackground method, and suddenly you end up with duplicate code.

Of course this wouldn’t have happened if your ‘createBackground’ method was called something like ‘createCustomRectangle’.

You can also see this example in reverse.

Let’s say you’re skimming through some API documentation. I struggle to believe you would actually read the implementation of each method to see if there could be anything that does exactly what you need.

The first instinct is always to look at the name of something and see if that can be used in the context you are trying to cover.

Conclusion

These are just some suggestions that I came up with after doing lots of code reviews and paired coding.

They may sound silly but I can guarantee that many coders make the mistake of underestimating them or taking them for granted, while others just don’t even consider them, causing a lot of unnecessary stress, not only for others but also for themselves in the future.

Also as they say, sharing is caring so I really hope that these suggestions help you!

Let me know your thoughts and happy Coding :)

--

--