Jason C. McDonaldLook at you, with your ready-to-ship project! You're quite proud of your style and standards conformance, and your strict adherence to DRY and SOLID. You've got tests, your bugs are squashed, your user and API documentation polished. Your code is so clean, it's shining!
I've got bad news. Your code base might still be terrible.
So, What's the Point?
To be clear, I'm not against anything I've described. The principles of TDD, DRY, SOLID (and a whole host of other fancypants acronyms) all have their place. Documentation, standards, and style are important. If you have actually managed to implement all of those, you deserve a serious pat on the back.
With that said, it is really easy to get caught up in the catchy acronyms and "Top Ten Ways To Write Clean Code," we forget why we're doing all this in the first place. And if you're not tuned into the purpose, all of the above will ultimately amount to null.
Regardless of purpose, language, or methodology, ALL software must fulfill exactly two criteria:
The software must accomplish its stated goals.
The software must be maintainable (thus, readable) by future developers.
The trouble is, to accomplish #1, the code need only to compile, pass tests, and work as expected. Even terribly-designed code can do that. We spend so much time focusing on #1, we often leave #2 to an afterthought. At best, we read a few articles on dev.to(), boil everything down to a few easy-to-follow rules that let us go on autopilot about maintainability. After all, making the software do stuff is a lot more fun.
But clean, DRY, SOLID spaghetti is still spaghetti. In fact, it's the worst kind! You probably know what I'm talking about: you boil spaghetti, drain it, and leave it sitting in the pan for heaven-knows-how-long. It's certainly clean, and by time you remember it, it's also an inedible dry mass. You can't do squat with a solid disk of spaghetti.
Similarly, an unmaintainable code base is doomed from the start. New features are hard to add. Bugs are almost impossible to fix. New contributors are scared off. Anyone compelled to maintain the code is sentenced to days or weeks of suffering as they pick and pry loose the program logic strand-by-strand from the tangled mass.
Before our code meets this unappetizing fate, let's look at a few ways our well-intentioned principles of maintainability go bad.
DRY Meets Wasteland
DRY: Don't Repeat Yourself
This handy little acronym is bandied about freely. It's a pretty obvious concept. Instead of writing the same code sixteen times, or even six times, put it in a function and call it!
There has to be a line, however. I've seen code bases where, to find out what one single-goal function did, I had to jump to no less than two dozen other functions and macros, across multiple files. To make matters worse, I wasn't even sure which of the hundreds of linked files contained the next piece of the call stack. Everything had been abstracted out to the nth degree, making the code base entirely unreadable.
SOLID's "Dependency Inversion principle" carries this same risk. We can abstract things out to the point where nearly every function is only ever calling other functions.
At that point, no one wins. Performance takes a hit (heard of "instruction cache misses"?) Readability nosedives. It takes far longer than it should to grok the code.
The fix here isn't simple. DRY is important, but it requires careful discernment. Before you abstract something out, you need to consider the cost. Then, you need to carefully determine the best way to abstract the functionality. Make it easy to follow the trail.
Cleaning the Crime Scene
I get it. You want a spotless code base, and who can blame you? It's easy on the eyes, and makes you look like a veritable programming genius. In the process of cleaning up, however, we tend to make some terrible mistakes.
Too Few Comments
Perhaps the worst is removing (or never putting in) intention comments. Many people make the argument that "comments fall out of sync with the code". Several standards proudly assert that comments should be rare, if present at all. "Write self-commenting code!" they exhort.
The trouble is, no code ever written can answer its own "why" to someone unfamiliar with the code. I firmly believe in, and actively practice, the principles of Commenting Showing Intent (CSI). In short, every logical block should have a comment describing its intention, its "why". We can't trust our intuition on this one, since almost everything is obvious to us at the time of writing. The fact is, no one can read your mind.
In answer to those who claim that comments fall out of sync, I believe that only happens if you let it. Implementing a commenting standard like CSI means you are using those comments as part of the code review process. A mismatch between stated intention and actual functionality should always be treated as a bug and resolved. Code without intention comments shouldn't even be allowed through to the code base in most cases.
Mind you, this doesn't mean you go whole-hog and clutter up your code with redundant comments restating the code. I've found it is usually best to comment everything first, commit, and then let those comments sit for a few weeks or months. Once your familiarity with the code has worn off, you'll be able to clarify and/or remove redundant, "what"-style comments.
Too Many Terrible Names
Self-documenting code is a wonderful thing. It allows us to see the purpose of the function or variable right in its name. The fact we need to follow this goes without saying.
But what about when it goes too far?
RtlWriteDecodedUcsDataIntoSmartLBlobUcsWritingContext();
If I had to maintain a code base of functions like that, I'd rather quit and become a full-time musician. (There are more if you have a strong stomach.)
Find a balance between descriptive names and readable names.
While we're on the topic, if you're using Systems Hungarian in your code, you need to stop right now. Before you do anything else, use Find & Replace and get rid of those horrendously useless type prefixes. That was never what Charles Simonyi meant it to be. (By contrast, Apps Hungarian is self-documenting naming in action. Look it up.)
Too Little...Everything
Second, clean code is often "concise" code, but it is very easy to go overboard here! While you could collapse an entire conditional statement into nested lambda expressions in a ternary operator, can anyone else even READ that?
Too often, we try to look brilliant at the expense of truly good code. Just because you can doesn't mean you should. Strike a healthy balance between concise and readable.
Too Little Whitespace
Lastly, please don't minify your working code base. Ever. Period. That's not clean, that's maintainability suicide. In fact, except for a specific, objective, verifiable business case on shipped code (e.g. optimization or obfuscation), it's just a bad idea in any context. Sooner or later, someone will need to read through that code to diagnose a bug or replicate functionality. Be kind - don't minify.
When SOLID Becomes Stupidity
In object-oriented programming, the SOLID principles are incredibly useful. However, none of them can be applied blindly.
The story you are about to read is true. Names have been changed to protect the idiots.
I once worked with a fairly popular and widespread web platform. I can say with fairly firm conviction that this code base followed SOLID principles to a "T". However, I am also adamant that the entire canonical source should be used as the test payload on an unmanned mission to the sun. It is, to date, the worst code I've ever seen. Yet, consider it in light of SOLID...
Single-responsibility Principle: Each class had one specific responsibility. And there were a lot of classes. As in, thousands.
Open-closed Principle: Every additional feature added to the code base came in the form of extension, and always via inheritance. Yes, always inheritance.
Liskov substitution principle: Every class could be substituted for a parent class, all the way up the chain. No exceptions.
Interface segregation principle: You only ever had to use the features you needed. Everything was single-inheritance based, so you never needed to import X just because you were using Y.
Dependency Inversion principle: Everything was abstracted. Literally everything. It was data-driven to the extreme.
Wow, 100% compliance with SOLID! And yet, it was an unmaintainable nightmare. Every single class was ultimately inherited out of the same family tree, sometimes hundreds of layers deep. To write one class, you had to read through 99 others first. Then, if a bugfix or optimization took place high up in the chain (which no one ever bothered to actually document, mind you), it broke everything below it. Thus, code that worked perfectly in version 3.2 would be entirely broken in 3.3.
As a result, contributors were afraid to improve the terrible code higher in the inheritance chain, for fear they'd destroy everything. Documentation was out of date before it could even be finished. Dozens of books were written on the platform, and were immediately useless by time they were published.
Was SOLID at fault for that? Absolutely not! It just goes to show, SOLID isn't some sort of magic bullet for maintainability. Its principles have to be applied with common sense and attentive discernment. Any coder with experience in OOP knows that such an inheritance structure as I described is poor design, but the large open-source team responsible for this project did exactly that. They had a completely SOLID, completely DRY code base, and yet I consider it a candidate for the worst production code in the history of computers.
TDD: Test Driven Disaster
I'll be honest, anyone who can manage 100% code coverage for their test is a genius. I don't think I come anywhere close. However, while I write tests as a rule, there is a reason I'm afraid to embrace TDD: I've seen too many code bases follow it right over a cliff.
The goal of your tests should be to detect bugs in your production code, but the goal of your code should not be to pass the tests! I believe TDD is in especial danger of confusing the two objectives. If you've already written your tests, you will instinctively begin to write your code to pass them, and completely overlook other things...like functionality, readability, and maintainability. It is possible to become so myopically focused on your unit tests, you simply code to check them off as "passed," never even seeing the terrible hash your code is turning into until it's too late.
As always, this isn't a slam against TDD, but rather a warning for its practitioners (and everyone else who writes tests). To prevent this sort of tunnel vision, I recommend the following:
Write your production tests blind, AFTER your code is written and apparently functional. By "blind", I mean "don't look at your code while you write them". On paper, break down what you remember your code should be doing, and shouldn't be doing. Make a list, and write a test for each part. Ensure each test has explicit pass and fail conditions. Be unforgiving. Do Terrible Things To Your Code.
In practice, this has done two things for me:
In knowing that my production tests don't exist yet, I focus on writing good code, instead of tricking my little predeteremined gatekeepers.
I catch a lot of bugs and design flaws this way...and I mean a lot. Well over half of my fatal bugs and edge cases surface through my blind tests.
If you embrace TDD wholeheartedly and write tests before code, I recommend one step beyond. Delete your original tests and rewrite them blind. Only allow yourself to note what functionality they're testing for, and not how they did it.
Style Over Function
Style standards are wonderful, and they make the code more delightful to read. However, even this can go too far. While we should follow our style standards, we need to be prepared to break with them when readability or maintainability dictates. Here's a few examples, and the principles they reveal:
Would breaking off at 80 or 120 characters make the line harder to parse? (Yes, these are rare, but they happen.) Grant yourself style exceptions when doing so will improve the readability and/or maintainability.
Love one-line, bracket-free conditionals? So do programming errors. Be careful of where you use stylish shortcuts. Sometimes brackets are just safer.
Are you having to hack the code to maintain part of your style? (Yes, this happens!) Stop it! Style alone should never require functional changes to your code.
Are you on a crusade against ternary conditionals? Conversely, do you love them because they make you look like an ALPHA hacker? Use the tools that best combine readability, maintainability, and functionality. Set your opinion aside.
Common Sense Not Included
There are a lot of infinitely helpful principles and standards that help us write good code...but don't believe the hype! None of them are The One True Answer To Maintainable Code. You will always need to be actively invested in the process of crafting good code.
In programming, spaghetti is bad enough, but SOLID, DRY, clean spaghetti? That's the hardest of all to untangle!
