Things to look for in a code review

🔖

Overview

Completeness

Code Style

Clarity

Explanations

Complete All Tasks

Completion of all of the tasks is essential.

Pass the tests

Do not rewrite the tests to ‘pass’ your code. Rewrite your code to pass the tests. Pay attention to the real goal here.

Tests are your friend and your guide. Let tests help you. When you work for an employer, will they find it a good solution if you “fix” their payments system by making any input a valid credit card number? If not, why not?

Achieves task in intended way

You should also always try to solve the problem in the “general” rather than in the “specific”.

For example, we could set the challenge

Print every element of an array to the console Test 1: let array = [“A”,“B”,“C”] Test 2: let array = [“1”,“2”,“3”,“4”] Test 3: let array = [“Up”, “Down”]

To make this test pass you could write

console.log(array[0]);
console.log(array[1]);
console.log(array[2]);
console.log(array[3]);

However, that would not work for every case that exists - only the ones that we’re writing. Your aim isn’t to just pass the tests, it’s to write good code that solves the problem.

A better solution would be

array.forEach((item) => {
  console.log(item);
});

Edge Cases

Inputs to some code can often be grouped into different categories.

For example, if you accept a string, there are some strings which are all letters, some which are all numbers, some which contain punctuation, the empty string, and so on.

Maybe your function accepts strings, but it always tries to get the first character from the string. In this case, the empty string is an edge-case - it’s something you may need to handle specially, or that you may need to reject with an error.

Some edge cases are valid data, and some are not.

Some names contain punctuation; some people may only have a single name; your code must handle these edge cases by accepting the names. If someone doesn’t enter a name at all, your code should reject this edge case and throw an error.

If your code doesn’t notice unexpected inputs, you might rely on data that doesn’t exist or are malformed. As your code gets more complicated, these problems get more difficult to resolve.

See Falsehoods Programmers Believe About Names for some interesting examples.

Good code should handle all of these cases so that they can’t happen - and if they do - they don’t make your program crash or stop working.

Validations are used wherever necessary. The code never breaks under any circumstances. User input is never trusted. Regardless of it being a negative, over-sized, invalid format, etc., every input passed should be processed, sanitized before taking it further. Every object is checked for its existence before accessing its properties.

More Reading

Small Functions

Writing small functions makes your code easier to understand and easier to work with. Ideally you should be able to describe what a function does in one sentence, without the word “and”.

Good examples: “This function returns the user’s favourite author”, “This other function Tweets the title of an author’s most popular book”.

Bad example: “This function finds the user’s favourite author and sends a Tweet with the name of that author’s most popular book”.

More Reading

Debug Logging

The code should not contain debug logging that was used during development. For example, any logs like

console.log("Does this work");
console.log("1");
console.log(array[1].object.name);

should be removed.

Error Responses

Not just the error messages, every response that is returned by the server must be properly handled. It should have necessary headers, response messages, error codes and any other necessary details attached with it in required format. All possible scenarios are tested to avoid timeouts, unresponsive UI, etc.

More Reading

Non-duplicated Code

Duplicated code should be moved into functions so they can be referenced from multiple places.

Having lots of duplicated code in your coursework means that it is harder to change or update your code.

More Reading

Commented Out Code

Commenting out code is a great way to debug your code but when you use it to keep code for later it can become a problem.

Instead of commenting out code you want to keep but not use, consider moving it to separate file. This will make it easier to keep track of the code you’re currently working on.

Having lots of commented out code makes your code harder to read and makes it easier for you to get overwhelmed and lost in your code.

Since we use Git to record all of the changes to our code you can always use the Git History to see everything that has changed.

More Reading

Dead Code

Dead Code is code that is in your coursework but can’t be reached or run by your program.

For example, a function that is never called is considered dead code.

Dead code should be removed from your coursework or re-written so it is used. Having lots of dead code in your coursework can make your work hard to read and understand.

More Reading

Coding Standards

All code written by Trainees at CodeYourFuture should follow our Coding Standard guidelines.

These guidelines are based on ones used in industry.

Guidelines are important in big organisations so that everyone writes code in the same way across a large application. It’s important that trainees get used to writing in a consistent style early on.

More Reading

Well Indented Code

Indentation is a fundamental aspect of code styling, and plays a large role in influencing readability. First of all, indented code is easier to read through than unindented code.

With unindented code, the overall structure of the code might be somewhat difficult to see. However, with indented code, the overall structure of the code jumps out at you. Tabs tell you that a line of code is inside a function, loop, if statement, or else statement. Knowing which parts of the code is inside what will become especially important when your code becomes more complex.

Good Function Names

Good function names are important so that code is easy to read and understand what it is doing.

A bad function name might be

function doWork() {}

A good function name might be

function removeCapitalLetters(){)

Sometimes if you can’t come up with a good function name it’s an indication that the function doesn’t have a nice, crisp focus and needs to be broken up or rewritten.

But it’s well worth the trouble finding the best possible names, since it makes your code so much more understandable and usable.

More Reading

Good Variable Names

Each variable is named so it is clear which variable is being used at any time. It is important to use meaningful names for variables:

For example,

let pocketMoneyPence = 2000;

means that the variable pocketMoneyPence is being used to store how much pocket money you have, counting the number of pence. Right now you have £20.

The name given to each variable is up to the programmer, but ideally a variable name should have meaning you can understand without needing to read any other code, i.e. it should reflect the value that it is holding.

When tracking numbers that have multiple possible units, we often include the unit in the variable name. Above, we used pocketMoneyPence, because if we’d just used pocketMoney it would be unclear whether 2000 meant £20 or £2000 (or possibly €20!).

More Reading

More Readable Code

The most important thing when writing code is to make it easy to read and understand, and as simple as possible.

An example of writing more readable code might be using a filter instead of a complicated for loop. This is better, because it is immediately obvious what you’re trying to do - filter an array - and you’re less likely to make a mistake using it.

For example,

const trees = ["Oak", "Birch", "Beech", "Elm", "Hazel"];

let treesBeginningWithB = [];
for (var i = 0; i < trees.length; i++) {
  if (trees[i].chatAt(0) === "B") {
    treesBeginningWithB.push(trees[i]);
  }
}

console.log(treesBeginningWithB); // ["Birch", "Beech"]

Could be written like this be easier to understand and read.

const trees = ["Oak", "Birch", "Beech", "Elm", "Hazel"];

let treesBeginningWithB = trees.filter((tree) => tree.chatAt(0) === "B");

console.log(treesBeginningWithB); // ["Birch", "Beech"]

As you can see, we’ve reduced six lines of code into one and it is immediately obvious that it is trying to filter the trees beginning with B.

Writing readable code is a big topic and one that you’ll get better at as you go on. We encourage you to read more about it.

More Reading

Hard Coded Values

Hard coded values with no explanation make code hard to understand and read in the long term. For example

getData("https://www.api.com/retrieve");
calculateScreenSize(1800);

Will be hard to understand when your codebase grows.

Instead use constants to store your hard coded values - this means the value has a name, which can be usefully descriptive. For example

let WEATHER_ENDPOINT = "https://www.api.com/retrieve";
let MAX_SCREEN_WIDTH = 1800;

getData(WEATHER_ENDPOINT);
calculateScreenSize(MAX_SCREEN_WIDTH);

More Reading