Highly experienced software engineers have an intuition for understanding code at a “meta” level. After reading and writing millions of lines of code, they can detect patterns in the code which are not immediately obvious to a less experience programmer. When I start reading a large codebase, I can often tell after reading just one or two files what the quality level of the code is and what the experience level of its authors are. This is a key skill for experienced engineers, because that kind of understanding of the code gives you a sense of the strengths and weaknesses of the code, and when you encounter bugs or poor performance you can deduce the likely causes and quickly jump to the relevant part of the code without having to read a lot of it first to locate the source. You can use your understanding of the programmer’s level of expertise to quickly deduce what kinds of errors they have likely made, and that acts as a clue which can quicly lead you to the source of the bug or poor performance. Colloquially, this sense is often referred to as a “code smell”. Generally this term is used in the case of poor code, but let’s expand the definition and say that all code has a smell, and it can either be a sweet fragrance or a putrid stench.
To programmers with less experience, this smelling skill can often seem like magic. It’s not something you can learn from a book or a tutorial. Nor is it a skill that is tested during interviews or programming challenges. But it certainly makes a massive difference in how you interact with code. Having a good nose for code smell can give you a big advantage in understanding a codebase quickly, in detecting patterns and predicting where problems lie. You can see more than just the code itself, you can understand how the author thought of the code.
In the recent age of generated code, code quality has declined steeply across the industry. Many programmers have stopped reading and writing code and have chosen the speed gains of prompting chatbots over the quality and reliability of carefully thought out code. I’ve even heard some programmers say generated code is just as reliable as handwritten code because they generate test suites to make sure the output of the code is correct. They claim that the code itself is no longer relevant since no one reads or writes it, and the output of the code is the only relevant surface area of interaction.
But I care about the quality of code. The code itself is the source of truth for any piece of software, and any output it produces is a derivative artifact. Large amounts of low quality code can produce an adequate output which passes automated tests, but there is no automated test which will tell you how easy or difficult the code is to maintain long term, to refactor easily, to adapt flexibly with changing business requirements. All of these factors are just as important as the output in professional software engineering.
If you are a less experienced programmer who has not yet developed a good nose for code smell, you are likely to struggle with generated code. You will have a hard time getting your bearings, and you will find it exceedingly difficult to understand how to maintain or refactor it. I cannot give you a better nose for code smells, but I can point you in the right direction of how to start thinking about code in a way that brings out the smells. After reading hundreds of thousands of lines of generated code, I have noticed some common putrid stenches which are relatively easy to spot once you start looking for them.
Slop Smell #1: Weird abstractions
Abstractions in code are chosen for convenience. If you have a program which only runs on Saturday and Tuesday and has a different behavior on each of the two days, you should use an if statement with just 2 cases. But if the business requirement changes and now the program runs everyday with the difference in behavior coming from whether the code runs on a weekday or a weekend, you now have 2 choices for abstractions. You can either add 5 more cases to your earlier if statement, or you can add weekday/weekend as a layer of abstraction over the current day of the week and use 2 cases again. Both approaches will produce correct output, but one will be much more closely aligned with the business requirement and will be easier to read, understand and maintain.
To use another example, let’s say you have a button which appears on both pages and popups in your your graphical application. A change in the design language now says that buttons inside popups should have a slightly modified appearance and touch/click behavior. You have several choices for an abstraction here. You can make your original button take a parameter for whether it’s in a popup or not, and then the code for the button itself decides its appearance and behavior based on that parameter. Or you can create a generic and unstyled button and then create two more variants of the button as “page button” and “popup button” which use the generic button as a starting point then apply the appearance and behavior in their own layer of abstraction. Or you may also choose to make the page button the generic starting point, and then the popup variant uses that as a starting point and “overrides” some aspects it for its own use. Once again, all of these choices of abstraction will produce correct output. But if you suspect that the design language may change again and more button variations can be added, it is obvious that some kinds of abstraction will be much more scalable, flexible and easier to maintain than others. There is no unit test you can write which will produce a pass/fail result on your choice of abstraction. It is a matter of your understanding of the code and the context of the business within which it lives.
Generated code often produces really weird abstractions. In the weekday/weekend example above, if you saw an if/else statement with 2 branches, one for weekdays and one for weekends, your intuition would be that the behavior of the code changes based on that that parameter. If you saw 7 branches, one for each day of the week, your intuition might be that the program has different behaviors for each day of the week, only to then read the code and come to the conclusion that there are indeed only two behaviors based on weekday/weekend. This choice of abstraction failed to encode the “intention” of the program clearly, and generated code often does this. But it also fails at abstracting in ways humans don’t often do, by not only failing to align the code with the intention of the requirement but in a way that is entirely misleading. With generated code, you might find 2 branches labeled “a” and “b” with the days of the week grouped inside, with no checks for weekdays/weekends in the code at all. You might be able to deduce that the person prompting the chatbot told it to code a certain behavior for Mon-Fri and a different one for Sat-Sun, and the chatbot failed to understand the weekday/weekend intention because of the wording of the prompt. Perhaps english was not the first language of the prompter and they did not know to use the weekday/weekend language to accurately encode the intention of their program’s behavior.
For the UI button example, you might find that the generated code has two completely separate sets of code for the page button and the popup button. Reading this code would lead you to believe that these buttons are drastically different and unrelated, which you would find to be incorrect once you referred to the design system. Or you may find that the chatbot made the popup variant of the button the “base” one and the page button the one that derives from it. This kind of abstraction would once again be misleading because the code would imply the wrong intention behind the design system. And in both cases, the next time the design system added another button variant, these abstractions would lead to more and more messy code to keep up, compared to an abstraction which was more closely aligned with the intention of the design system which would be able to add new button variants with minimal code.
Slop Smell #2: Unreadable naming conventions
Giving clear names to your functions and variables is a key part of writing high quality code. Once again, encoding the intention of the program into the naming makes is easy to read and understand.
If you saw a piece of code like this
x = a * b * c
you would have to find more context clues to figure out what this math is accomplishing and why. If instead the code was
interest_payment = principal * interest_rate_annual * years
it would be obvious at a glance exactly what this code does. Both lines of code produce the same output, but one takes much more cognitive effort to understand. And there are no unit tests which can give you a pass/fail score for the cognitive load produced by a piece of code.
You might think this is an unusually egregious example, but I have seen generated code exactly like this and even worse. I have seen not just single lines of code which are impossible to understand without further context clues, but entire code blocks, functions and even whole files and modules with naming exactly like this. One line of cryptic code is managable, an entire module of cryptic code with hundreds or thousands of lines like this are almost impossible to read and require an immediate refactor to give it any hope of maintainability.
Slop Smell #3: Ghost code
Deployed production code often undergoes continuous change. At an earlier stage of its lifecycle, it changes to accomodate changing business requirements. Later down the line, it changes when the codebase is refactored for cleanliness and streamlining. Once the code reaches a level of maturity, it changes less frequently, and usually for performance or bugfixes. Sometimes certain parts of the code are no longer needed due to a change in the business requirement, or due to another part of the code having been written to take over certain functionality and this part of the code is no longer invoked after the new one takes over. Code that is no longer in use like this is called “dead code”, meaning it is no longer invoked from any part of your codebase. You can write a kind of test to detect dead code called a “code coverage” test. But there is a kind of unused code which cannot be coverage tested. I call it “ghost code”, and in my 10 years of programming I’ve only ever seen it in generated code.
Using the UI button example from above, suppose you saw in the code that there are 3 kinds of buttons: page buttons, popup buttons, and header buttons. But the program has no header in the UI. The header button is “dead code”, it is not invoked anywhere and code coverage tests will detect it. But instead if you saw only a single kind of button which accomodates for different kinds of appearance and behavior based on 5 parameters, and the code isn’t structured in a way which immediately makes it obvious these 5 parameters trace back to 3 kinds of buttons, of which only 2 are in use now, the unused header variant is not a clean piece of “dead code” which can be spotted by the human eye or automated tests and is instead “hidden” behind other parameters. It has contributed added lines of code and increased the complexity and cognitive load of understanding and maintaining the codebase. There is no header to be found in the program but it has left behind its ghost.
This is part 1 of a series. More example of slop code smells will follow in future posts.