Writing Quality Code, Part 1
Code styles and standards are highly overrated
Read the first blog in this series: Don't skip, Hot take.
Alex has been working in the field of software development for quite some time and has built up a lot of experience with code patterns, best practices, and other code quality concerns. Today, Alex is starting a new job as a lead software engineer at Etrain, a company that is developing software for in-house and online workshops. To start, Alex will join a small team of three other developers working on the internally used booking system. The team is working mostly unsupervised. Reportedly, things are not going well.
After introductions and some familiarization, Alex looks around the application's code base and asks the team about the biggest problems they are facing.
The team members are all in agreement. The code quality of the code base is awful. Alex, having seen the code, can’t really disagree. The next day, they request that Alex, as the new, experienced lead, set up some coding standards and style advice for the rest of the team. They bring up several issues.
The code base, they say, has multiple implementations of the same functionality (in the form of methods or classes that do roughly the same, but are in different locations). “This breaks the DRY principle, doesn’t it?” asks a team member.
“My biggest problem isn’t DRY,” interjects another colleague. “Our code is massively inconsistent. Not just in tone and organization, but also in syntax and naming conventions.”
“And that goes double for our git commit messages,” the third one adds.
“That’s nothing compared to our code bloat.” That’s the original team member again. “I bet those inconsistencies of yours will almost completely disappear if we remove all the outdated hacks and unused classes and methods.”
Another laughs. “True, but we’d probably double the number of bugs, and those are already considerable. Although bugs, I can stomach. It’s the typos that annoy me.”
After a bit more discussion, Alex asks the team a simple question. “What damage is this lack of styles and standards actually doing?”
The arguments for styles and standards
Alex’s question is – at least in my opinion – the only relevant one. The hot take of this blog is that code styles and standards are highly overrated. I’m sure you deduced that already, as it is the subtitle, but it bears repeating. Before trying to defend that statement, I think defining it is necessary.
A code style is a set of rules that prescribe how to capitalize and/or abbreviate the names of classes, methods, and variables, how to use tabs, whitespace and indentation, when and how to use comments, and other presentation-centric elements. The goal of following a code style is to make it easier to understand the code and to prevent errors. Code style deals mostly with consistency in presentation.
public class UserService
{
readonly UserRepository userRepo;
public UserService(UserRepository userRepo)
{
this.userRepo = userRepo;
}
public async Task Add(UserModel User)
{
await userRepo.CreateAsync(User);
///Commit the changes
await userRepo.CommitAsync();
}
}When reviewing the C# code above, you could make the following changes based on code style suggestions:
public class UserService
{
private readonly UserRepository _userRepository;
public UserService(UserRepository userRepository)
{
_userRepository = userRepository;
}
public async Task AddAsync(UserModel user)
{
await _userRepository.CreateAsync(user);
await _userRepository.CommitAsync();
}
}As you can see, it doesn’t change the functionality, but it does change how the code is presented. Interestingly, several of the changes are kind of debatable. Writing “repo” is certainly shorter than “repository”, and maybe we should prefer that instead of the longer, non-abbreviated phrasing. Adding an ‘Async’ suffix to the names of async methods is a nice idea if you need to specify that some methods are async when others aren’t, but what if almost your entire application is async? Does it still serve a purpose, or does it needlessly clutter your code base with buzzwords? And likewise, do we need to add that access modifier (private), or is it a needless filler?
Continuing from there, a code standard is a set of rules that prescribes how code should be written. This is broader than a code style, and some code standards may even expect developers to use a certain code style. A code standard guides or forces developers to make choices suited to the programming language. In C#, a standard will have notes on code reuse (e.g., DRY), code principles (e.g., SOLID), scope and hiding of implementation details (i.e., encapsulation), and many more. A code standard is set up to provide consistency in design. Microsoft, for example, has its own guidelines, found here.
To illustrate: after applying a certain code standard, the code above would look like this:
public class UserService : IUserService
{
private readonly ILogger<UserService> _logger;
private readonly IUserRepository _userRepository;
public UserService(ILogger<UserService> logger, IUserRepository userRepository)
{
_logger = logger;
_userRepository = userRepository;
}
public async Task<DataResult> AddAsync(UserModel user)
{
try
{
await _userRepository.CreateAsync(user);
await _userRepository.CommitAsync();
return DataResult.Success;
}
catch (RepositoryException ex)
{
_logger.LogError(ex);
return DataResult.Failure;
}
}
}One important way in which a code standard is like a code style is that the value of a given code standard is very dependent on consistency. Logging predictably is intended to help with debugging and monitoring, and the use of abstraction through interfaces will help with decoupling, which makes it easier to write unit tests and change the implementation of a class (and its dependencies) later. This is only true if this is done consistently throughout the code. In that case, it greatly increases a given developer’s efficiency, as there are clear rules for their own and others’ code, which prevents spending mental effort on decision-making when writing code, and on comprehension when reading code.
Given these definitions, styles and standards create consistent, readable code that follows agreed-upon best practices and makes developers work more efficiently, while simultaneously preventing bugs. That’s great, right? It begs the question: how could introducing and maintaining code styles and standards ever be a bad thing?
The arguments against styles and standards
My first argument against styles and standards is one based on my own experiences in the field. Styles and standards often become goals in and of themselves, which I find baffling. They are methods. Not goals. At least, not goals that anyone but software engineers themselves care about. A relevant goal would be: we want our software development process to be flexible.
And that is a lofty goal. With the only constant in software development being constant change, it is very important that writing new code and updating existing code is easy and quick to do. While you could make the argument that code without a consistent style and standard will be very hard to maintain, I would argue that in most situations, the exact opposite is true. Rigidly enforced code styles and standards can very easily cause inflexibility and make it harder to implement new, unforeseen features. Being constrained by a required specific style and the same rigid standards, regardless of context, will cause efficiency to drop, especially once the styles and standards become outdated or at least non-optimal for the problem at hand.
What about bugs? Styles and standards prevent bugs, right? I think they do, yes, but indirectly. And also, they don’t. On the one hand, consistent code sometimes makes it easier to spot an error in your logic and thus, helps locate bugs. On the other hand, if you spend all your time focusing on styles and standards, you leave less time to focus on finding or preventing bugs. And people only have so much time and a limited attention span.
Also, let’s be fair. Many so-called bugs cause no real-world damage. They’re just annoying, or more often, theoretical problems with edge cases that never actually occur. But even if we’re talking about bugs that are actual, tangible problem-causers for actual people… how often would they have been prevented because of styles and standards?
Finally, if you look at the time some teams spend on those topics, you’d think styles and standards are the be-all and end-all of bringing value to your code. Heated discussions on the what, how, and why of capitalization; hierarchical interlinked documentation wikis on the standards that were agreed upon after hourlong meetings; refactoring (essentially problem-free) code for the sake of consistency; and, one of my biggest pet peeves, pull requests where the only comments are about tabs and naked ifs. Please, please, PLEASE: if you reserve some of your precious time to review someone’s code, don’t spend it discussing grammar or theoretical pattern checklists. Spend it on the actual perceived risks and rewards of the code.
I’d like to offer up a metaphor. Doctors stereotypically have terrible handwriting. But that doctor’s recipe that they give you after a consultation? That is still high-quality medical insight. Is the terrible penmanship annoying? Hell yes. But don’t confuse a messy presentation with a lack of veracity.
Measuring the damage
If using styles and standards causes problems, and not using styles and standards causes problems, what can we do? Well, why not find the golden center, one might say. Be pragmatic about it, and only apply styles and standards when they fit the problem. Some parts of code are more important than others. It makes no sense to hold them all to the same standards, right?
When the expectation of a certain style or standard meets the absence of that style or standard, it can cause even greater problems. It’s like outdated comments or tests that falsely pass: if developers assume a certain style or standard is present, and it is unexpectedly or unclearly missing… the assumption may lead to grave errors. Instead of pragmatic, opportunistic styles and standards may be the worst of both worlds.
So, neither A, Z, nor any letter in between seems the right call. Where does that leave us, software engineers? Well, I’d say that your first instinct should always be to measure before you act, which matches well with the Agile and DevOps mindsets that I am quite experienced with.
Measure the time required to adapt to new features and functionality. Measure the Mean Time to Change (MTTC), the Mean Time Between Failure (MTBF) and Mean Time To Repair (MTTR). Measure user approval scores and measure the software’s correctness. And, most importantly, determine what passing scores for those measurements are. That’s what hour-long code quality meetings should be about.
Let’s say that you and your team have gathered all those measurements. That’s when you can answer Alex’s question at the start of this blog. What damage is actually being done? And what can we do to fix it?
My solutions
So, from the paragraphs above, you might have gotten the impression that I do not believe in code standards at all. And that is – somewhat predictably, not the case.
Firstly, I believe in standards in processes. The way a software engineer determines the quality of some code needs to be standardized in some way (e.g., code reviews via pull requests). But, during code reviews, do not review the code. Review the functionality. Think about the edge cases. Approach a code review where you and the author are equaled (because, hello, you are). You’re trying to improve the code together. The worst way to approach a code review is as if you were a teacher marking a paper, or an auditor going down a checklist.
Secondly, I think styles and standards can help as long as we don’t waste any manual effort on it. Use automation. If you can apply a set of styles on save, do so. I think the specifics of the style set you choose are almost irrelevant; with automation, you can apply the styles consistently and that is a major benefit. For standards, this may be a bit harder, but there are plenty of tools that will perform static code analysis (e.g., SonarQube and SonarCloud). These tools will automatically mark problem areas, not based on the ideas of your team, but on the ideas of the wider community. And usually, your IDE – whether it’s Visual Studio, Rider, or Eclipse - will allow a quick and semi-automated way to refactor the issue. This process does not require a reviewer and should ideally be performed before the formal, manual code review, minimizing effort. If you cannot apply a standard in a fully automated or at least semi-automated manner, I would refrain from naming it a standard. Consider it as an option, yes. But in the end, when setting standards that require a lot of manual effort, the cons outweigh the pros in such cases.
Finally, and most importantly, my only actual standard for code quality. Any code that is part of a business application should be easy to debug (manually) and able to be tested (in an automated manner, i.e., unit tests). I feel that if you manage to follow that standard, other styles and standards are just way less impactful. Code that is hard to debug may need easier-to-reach entry points (and hey, a unit test may be just such an entry point, how convenient is that?) or is just too complex or too abstract. On the other hand, code that is hard to test is often either too simple or not abstract enough. And just as an aside, the easiest way to make sure your code is testable, is to make it tested. I’m not saying you need to aim for 100% code coverage here (that just invites a lot of rigidity again), and yes, bad or outdated tests can be very dangerous, maybe even more so than no tests… but unit testing is the most efficient way to invite and maintain quality in your code. It is so important, that automated tests will be the major star of the next entry in this series. So, forget about styles and standards, as long as you have tests. How’s that for a hot take?
Fixing the booking application
After a time of analysis and measuring, Alex’s team identifies a few problem areas where the rigidity, the number of failures and the severity of errors are unacceptable to the team or the business. First, some bugs are causing real damage, either to end-users or to the development time. They are made top priorities to fix, while other bugs are allowed to roam around for a while. The team refactors the problem areas to be easier to debug, choosing solutions that fit the context of each instance. Alex configures a style set that other teams at Etrain are using. The style is automatically applied when saving a code file. If they ever want to change to a different style set, that will be almost no effort at all. Alex also configured a static code analysis tool for the booking system, which will automatically flag code that has potential risks. The tool is not used as a gateway, but as an automated source of reflection for the author of the code; an artificially intelligent rubber ducky if you will.
“Now, we need to start writing some unit tests,” Alex says during an online meeting.
“Easier said than done, though,” another team member interjects.
“Don’t worry,” Alex answers while clicking the ‘share screen’ button.
While the rest of the team watches along, Alex creates a new test project. “Virtually everything a software engineer does is easier said than done. It hasn’t stopped me before.”
An overview
For a glimpse into the future, an overview of what has already been posted and what is still to come:
Contact me
As always, I ask you to contact me. Send me your corrections, provide suggestions, ask your questions, deliver some hot takes of your own, or share a personal story. I promise to read it, and if possible, will include it in the series.
Writing Quality Code, Part 1
