One of my favourite parts about being a software engineer is reviewing code. You get to learn alot about an individual from the code that they write and also learn how to potentially do something more efficient or understand a component of the application you had little exposure to before. If I look at a merge request, within the first few lines of code that is submitted I can get a good sense of how many years they have been programming and if they know what they are doing or if they just copied and pasted something from StackOverflow.
Graduate software engineers, entry level software engineers, or individuals who have just started programming, will often leave out some crucial elements such as appropriate null checks, logging, comments or appropriately named classes, methods and variables. There is nothing worse IMO then looking at some code and seeing a variable named “numbers”. It is most likely some collection object like a List and contains numerical values but offers little context in respect of the application it is used in. It may make sense to the person who wrote it but it probably won’t to others and you will most likely forget its context several months down the line. Red flags are very easy to spot here.
The more experience you get, the quality of the code that is submitted is generally far easier to read, is structured and broken down logically, has been optimised, there are the appropriate exceptions and null checks in place and the classes, methods and variables are named appropriately. But to get to this level you not only need to write a good amount of code but also look over and see how others are structuring and writing their code and do reading/research in your own time.
At work when I get a request to do a code review there are a number of elements I look at. Some of which include:
- Appropriate and meaningful class, method, and variable names.
- Small, compact and single purpose methods.
- Appropriate null and boundary conditional checks.
- Some level of optimisation on the operations performed.
- Meaningful unit and/or integration tests have been implemented.
At times I think that I am fairly harsh on the individual who submitted the code to be reviewed but then I think to myself that the better the code repository is in, the easier it will be to maintain and if good code practices are enforced then it will make everyone a better programmer. If I didn’t start being critical of my code and trying to better myself then when I do the code reviews I probably wouldn’t be as critical of theirs. Setting a high standard will only make the lives of software engineers working on the project much easier in the long run even if there is short term pain.