In the Beginning
Early in my career I discovered, like many others, Uncle Bob Martin and his clean code movement. I had an excellent mentor in my first software job, who showed me the value of code reviews. I realised their significance for improving code quality and as an essential tool for learning and knowledge sharing within the team. In over 20 years of developing I have written a lot of code, some of which I returned to later with a more experienced eye. Increased experience, feedback from peers and continual learning helped develop me as engineer. And that development doesn't stop. The early career learning on clean code and reading Effective Java by Joshua Block, have really stuck with me in the subsequent years. I am writing on this personal site to help formulate my own thinking more clearly, archive my career learning and to keep exploring clean code and clean design.
First, I want to look at the very basic things I look for when approaching a piece of code either as part of making updates to it or as part of a code review. These simple steps I will expand on in further posts. Experienced developers will know these but lets start with some basics.
Well Formatted
Over the years code comparison tools have got better at identifying white space changes versus lines of code changes. Early in my career, I would be warned against reformatting code as this would mess with the history of changes. Sometimes a compromise was reached which allowed format only changes in the version history. This may have been particular to the senior engineers I was working with at the time but in general in my opinion badly formatted code is challenging for long term maintainability. So, reformatting is perfectly acceptable to help with that.
Code can wither on the vine and often needs to be tended. An absolutely basic essential is that code format is consistent. End of line characters due to being updated on different operating systems or by differently setup editors can sneak in. The originating engineer doesn't even see it but another editor or code comparison tool will display those characters differently. This is mainly a problem of the past but you may open up some code one day and wonder what is going on with it. Control M characters might be lurking.
Apply a Standard
Your editor of choice is your friend here but within a repository the formatting should be consistent. We all know developers have their own personal choice on their preferred format but agreeing a standard one and sticking to it is vital. Then, set up your editor to this format and make it easy to set-up for anyone joining the repo. It has to be non-negotiable for your code base.
In combination with linting tools, the repo code formatting should be a no brainer and time shouldn't be wasted in code reviews pointing out formatting issues. Hopefully, in a polite and courteous manner, I would send a code review back asking to format it well first before continuing with the review.
Repo Strucuture
Also, in this area is a good structure for the repo folders. Keep it simple and intuitive, and I also think it's good for maintenance, to explain the folder structure in the README file especially if there is a reason for something being non-standard from the community expected format.
That leads nicely onto a README file. I don't think I ever read anywhere that should include a README file at the root of a project but they have become standard practise. It's a simple thing but explaining the repo, how to build, test and run it, really helps the next developer get up and running quickly. And of course, keep it up to date as things change, which they will in an active code base.
All in the Naming
I have seen some interesting coding standards over the years when it comes to naming and in a long running code base developers will endeavour to keep to the standard even when they themselves think it unusual. I won't quote any particular interesting naming methods I have seen on the past but suffice to say naming is very important for code clarity. I would encourage anyone to read Uncle Bob Martin on this in particular because it fosters deeper thinking about naming in code. Choose your names thoughtfully, communicating the intent as carefully as possible.
As mentioned above naming is not just important in the code but in the structure within the repo. There will always be opinion on what is good but consistency is always so important.
Conditional statements
In code reviews, I wouldn't necessarily provide feedback on other developers use of conditional statements unless it was particularly out of control. And often they are making changes in an mess not of their doing. In this area I try to lead by example with the changes I make and code I design.
Conditional statements are hard to avoid but also their overuse particularly with many conditions, and deep nesting increases the complexity and test-ability of code. When I write code, I continuously ask myself how a piece of code could be made simpler and can any conditional statements be reduced or eliminated. Multiple clauses and many nested loops and conditions are a clear sign to me, that the overall design could be improved. The balance must be struck but in this area in particular, code can be refactored to improve the design and I aim to provide a good example of how to do this in a future post.
Boolean as Parameters
Leading on from conditional statements one lesson I learnt much later in my career was using boolean parameters in method signatures. When you break it down it is essentially two different methods. Common parts can be extracted out. At some point that boolean will be evaluated with a conditional statement and will execute differently. Using the principle that a method or function should do one thing, passing booleans overly complicates the method. It also means it's doing more than one simple thing. I have reviewed code where the same boolean just gets passed from method to method before somewhere down the line in the path it gets used. In this case, we have a serious design issue to fix but a good rule of thumb is; don't pass booleans as parameters.
Write Unit Tests
I am sure software engineering university courses are teaching this but write unit tests from the beginning as shifting the needle on this later on is so difficult and time consuming, that it won't get done once the pressures of making further updates and changes to the repo kicks in.
Use coverage tools in your build, and set high coverage rules but this will only really matter if your unit tests actually test the code and not just exercise it. For the benefit of your future self and the next maintainer of the code base write meaningful tests. I also believe that thinking carefully about unit tests and how to test a piece of code will help the overall design too. It will challenge you to think about the actual implementation in my experience.
Conclusion
These are just a few of the basic things I look for when I approach a piece of code either when making changes or from performing a code review. All opinions are my own.