Note for the easily offended: I know that these blogs are supposed to be about beautiful code, but I believe that by exploring its opposite – nasty, ugly, horrific code – we will enrich our discussions of, and appreciation for, code beauty. I am also aware that the opposite of beautiful is ugly, not crap; and I realize that the term crap is mildly vulgar. I decided to use the word anyway because that’s the term most programmers use when referring to offensively ugly code. In other words, I heard the expression “This code is crap!” much more often than “This code is ugly!” Besides, Wikipedia says that “… [crap] is considered mild enough that even a child may use it informally.” So there :-).
Hell is other people. - J. P. Sartre
Hell is other people’s code. - T-Shirt seen recently in Mountain View, CA
While writing my chapter for Beautiful Code, I was forced to think beyond code functionality and efficiency. I had to think about beauty in code. And thinking about what makes a particular piece of code beautiful also made me think about the complementary question: What makes a particular piece of code crappy?
In the last few months, this question has led to heated conversations with colleagues, friends in the business, and random software engineers sitting next to me at the local coffee house. It would be naïve to expect programmers to unanimously agree on anything, but on the subject of crappy code most people I talked with agreed on a few of things:
First. Crap is usually another programmer’s code. Most programmers have a pretty high tolerance for working with less-than-beautiful code – as long as they were the ones who wrote it. That’s either because they think their code is beautiful – in the same way every parent thinks their children are beautiful – or because it’s ugly in a way that they are familiar with. But their tolerance level drops dramatically when they have to deal with someone else’s code. Unfortunately, most developers spend most of their time working with, and maintaining, other people’s code - which explains the next area of agreement.
Second. There’s a lot of crappy code out there. Most of the underlying code in the software that runs our world is at the opposite end of beautiful. It's important to note that bad code does not necessarily imply bad or useless software applications. There’s a big difference between software and code. Software is the finished working product; it’s what the end user sees and experiences. Code is the guts of software; it’s what the programmer sees and experiences. These are two very different experiences; like the experience of a diner enjoying a sausage, and that of the meat plant worker making that sausage. It’s not only possible to have very useful and successful software that’s built on crappy code – it’s the norm. Which leads to the next area of agreement.
Third. Crappy code is expected and tolerated. Most software organizations don’t seem to care much about how bad the underlying code is – as long as the software does what it’s supposed to do and it makes money. If they did care they would have better measures to prevent nasty code from contaminating key components of their precious applications – a quick glance at the code in most software will show you that that’s not the case. But at some point, if the product is successful, they will have to care (see next point).
Fourth. Crappy code becomes a real big issue when it changes hand and has to be modified. At some point or other some key developers decide to leave the company – often because they can no longer stand to work with the mess of code they have created. That’s when the search for victims to inherit and maintain the code base begins, and that’s when crappy code rears its ugly head and has to be dealt with.
The bottom line is that most people agree that the real big problem with crappy code manifests itself when the code has to be changed. And the problem gets much worse if the original developers are no longer around – or haven’t worked on the code for a while so they can’t remember what they were thinking at the time.
Since, eventually, most successful software will change hands and will have to be updated and maintained, the problem of crappy code is a serious one. This raises some interesting questions:
Is crappy code easier to identify than beautiful code? Beauty might be in the eye of the beholder, but is ugliness is universally recognized?
Are there some common characteristics of code that would make most developers who have to inherit it and maintain it say: “Oh crap!” when they see it?
Are those characteristics quantifiable and measurable?
If so, we could build a crap code detector. Wouldn’t that be fun – and possibly even useful?
As you might guess, I’ve been thinking about the problem of crappy code for quite a bit recently. I’ll be sharing my thoughts in the next few blogs; I’ll even go out on a limb and propose a way to detect crappy code.
But in the meantime, what do *you* think? What code characteristics would make you say: “Pardon my French, but this code is crap!” What code would make you recoil in horror at the thought of having to maintain it?
Alberto
P.S. An earlier version of this blog entry appeared on Artima a couple of months ago, before the Beautiful Code blog was started. You might want to check some of the comments it elicited at the time: Pardon My French ... comments on Artima


Comments (24)
It seems to me that the more difficult it is to understand code, the more crap it is.
Other possible factors:
- Need to consult code snippets more than 20 lines away and/or in another file in order to understand a line of code.
- Quality of documentation of the specs governing the code.
- Overall size of code (crappy 20-line function is less crappy than clean 20,000-line application).
Posted by TDDPirate | September 5, 2007 8:19 PM
Michael Feathers MC'd a great session at Agile2007 where he handed out 120 snippets of code and had us grade them - in a blink test - as beautiful or crappy to see if there was any consensus or any common rules for identifying code as good or bad.
He promised to start a mailing list to continue the exercise...
....Michael?
Posted by Kevin Lawrence | September 6, 2007 12:14 AM
These are some of my "crap" indicators:
- No error detection/handling.
- Incorrect or non use of language features/standard libraries.
- Non understanding/incorrect use of programming paradigms (e.g. "is a", "uses a" ...).
- Useless, lack of, or bad documentation.
- Massive functions or methods
- Obfuscated code.
Posted by Roger | September 6, 2007 1:42 AM
From the top of my head I would say:
- when I have to read a block of code about 5 times in order to understand what it does.
- when there are more than 2 instructions in a line.
- when in order to understand what a global variable does I have to search the code instead of just looking at the name or a commentary that it should have.
- when I see something like: offset = 15 + 3 + 2;
(and yes, I saw this lots of times. Even offset = 15 shouldn't be used in my opinion).
I'm sure there are lots more.
Posted by Cristian Cornea | September 6, 2007 3:31 AM
Crappy code ...
- has poor error handling and fails badly, leaving modules in an inconsistent state;
- make assumptions and has side effects that could not reasonably be expected;
- has convoluted flow, forcing me to think.
Posted by Twylite | September 6, 2007 9:25 AM
Here are the things that I identify as "crap" code:
1. Not using naming conventions - As IDE's get more sophisticated, this becomes less of an issue, but when dealing with older code, adding "str" in front of a variable being used for a string makes sense.
2. Using On Error Resume Next (or other error skippers) when not needed. Also, using error trap code when it isn't necessary. (I don't subscribe to the notion that EVERY procedure requires error trapping)
3. Over-creative code - why use ten statements to do something when three or four will do? This includes declaring too many variables or objects.
4. Inappropriate use of object scope - using global level variables instead of procedure level variables just to save from having to declare them locally.
Like others, this is an incomplete list, but these are things that I didn't see previously mentioned.
Posted by W Scott Grant | September 6, 2007 12:09 PM
As another corollary is there such a thing as average code? I think perhaps by virtue of it not being part of the overwhelming majority termed crappy it gets included with the beautiful.
A couple ugly things I have seen recently which no one mentioned yet:
- Commented-out code interspersed in live code (in a world with svn/cvs this should never be.)
- Redundant code. One guy does all the work to accomplish "a" then another comes along and does it over again somewhere else or in a slightly different way.
- Duplicate code. I have seen it where one block of code is literally copied verbatim to multiple places instead of refactoring it to a common area.
- Gigantic classes.
- Out-of-date documentation. This is sometimes worse than none at all.
I also have to disagree with Cristian on a couple points. First off, I have seen people who can't understand some algorithms after reading them 10+ times; that doesn't mean it's a bad algorithm. Second, your average "for" loop puts more than two instructions per line (as do a few other things)-- you may want to clarify that complaint a bit. I know what you mean, I just think saying a specific max number of instructions per line is not the best way to describe it. As for your third point in my opinion having global variables at all is a bit ugly... Your last point about magic numbers I agree with fully though.
Posted by Jason Andorfer | September 6, 2007 12:56 PM
I hate it when:
Someone recreates functions because they were too distracted/overworked/lazy/dumb to see if a working function already existed.
Someone codes 5 lines to do something that could have been done with 1.
There are no naming conventions.
Someone uses one name for an object in one section of code but a different name in another but the two sections relate or interact. Duh!
There is no way to know who wrote the code or who made changes to it.
There is no formatting or inconsistent formatting.
Reiterate:
"more than 2 instructions in a line" is crap.
"no documentation or poor docs/comments" is crap.
Posted by Matt | September 6, 2007 2:40 PM
"Crap" indicators:
1) Echoing Twylite: Code that has side effects
2) Echoing Roger/Twylite: Error detecting/handling consists of suppressing errors
3) Echoing Roger & Jason: Long methods or classes
4) Expanding on Twylite's points: Makes no tests for validity, meaning it will still produce output when the inputs are outside of the designed bounds.
5) Contradicting W Scott: Using useless naming conventions. using m_ or str or any other identifier of WHAT the variable is in a variable name means refactoring involves changing the variable name, unnecessarily causing a massive check-in that may bridge many files, if the item is global.
6) Contradicting W Scott again: Over-creative code: Why use three or four statements that wrap across 10 lines and may get very strange formatting (hello, Eclipse, I'm looking at you) when the more statements will identify the intermediate variables (for clarity) but will be trivially removed by a compiler.
7) Echoing W Scott: Inappropriate use of object scope
Posted by Tom P | September 6, 2007 10:42 PM
Perl::Critic is a sort of crap detector for Perl. They started with the best practices for Perl laid out in "Perl Best Practices" and made a utility to scan code for violations. It will even tell you the page of PBP to look on for an explanation!
$ perlcritic lib/Test/Simple.pm
Expression form of "eval" at line 8, column 12. See page 161 of PBP. (Severity: 5)
Subroutine prototypes used at line 80, column 1. See page 194 of PBP. (Severity: 5)
It has varying levels of severity (1 through 5 or brutal, cruel, harsh, stern and gentle), configurable per project as well as per rule and you can plug in your own critiques.
It's a much lower level syntactic level crap detector then what was eluded to in the original post, but its something.
Posted by Michael Schwern | September 7, 2007 12:52 AM
SIlly variable names: extra m_ and underscores don't help me understand what a variable stores.
This alone makes the STL nearly unreadable, for instance.
Error checking is good, obviously, but I think checking malloc() is really silly. If it fails you haven't got any recovery case unless you're doing embedded programming.
Posted by Alexander Strange | September 7, 2007 3:03 PM
i think people should stick more to conventions and
WAYS THAT WORK...
there are SO many examples of code i hate
its often easier to write from scratch than reuse existing code (i mean for "script" languages like ruby)
Posted by Anonymous | September 7, 2007 6:22 PM
It is all, of course, in the eye of the beholder.
My personal taste abhors:
- poor indentation which obfuscates nesting levels
- mindless following of a style convention when a little thought would yield a tabular form that makes the pattern of similarities and differences readily apparent
- the obvious: overly clever (and therefore unnecessarily complicated) algorithms
- "blind" actions, such as subroutines which modify locally used global variables for no reason
Posted by Joe | September 7, 2007 6:31 PM
> What code characteristics would make you say: “Pardon my French, but this code is crap!”
This is actually one of my favorite interview questions. I think lots of you guys already nailed the big ones: Functions with side effects. Big classes. Low cohesion. High coupling. Cut-paste-tweak duplication. Meaningless comments. Premature optimization. Premature generalization. Nonexistent error handling. Error handling everywhere. Empty catch blocks. Lots of "if" statements (special cases). Shared state concurrency.
I'll buck the trend and say the following aren't code smells, however:
* m_ notation. There are many abuses of Hungarian, but this isn't one of them.
* Multiple instructions on a line.
* Superficial layout inconsistencies. I don't think code has to cater to OCD sufferers to be considered "readable".
* Few comments. Well-written code doesn't need a lot of comments. (No comments at all is definitely a problem, but it's also one I've never run into. I have seen lots of problematic heavily commented code, though).
* Long functions. I look more at the number of locals and the level of indentation. Sometimes you really do need to do A, then B, then C, all the way to Z -- and in many cases, arbitrarily chopping it up into funclets is actually the wrong thing to do.
Posted by cashto | September 7, 2007 10:12 PM
I have once worked on crappy code maintaining it. So this list is from that application.
-No documentation in the form of inline comments or global architectural documents or any other form of documentation such as the definition of the database model(there often were several columns with the same meaning and value but different names)
-Using an esoteric scripting language with a lot of bugs in the runtime(Powerscript)
-Using multilevels of inheritance(more than 3) and at the last level, stop the inheritance and redefine everything again(this is not your usual inheritance, its specific to Powerscript)
-About 15 people working on this application over 7 years and each with a different style and different naming conventions and none were left in the company
Posted by Alaa Salman | September 8, 2007 5:36 AM
@Jason Andorfer:
I agree with your corrections, so let me explain myself a bit.
- multiple instructions on a line
This is ugly coding when there are unnecessary multiple instructions in one line. I agree that maybe there are cases where more than two instructions in a row are needed, but not when you could easily put one instruction after the next.
- hard to understand code
I agree that some algorithms need bits of code that aren't that easy to comprehend at first. I'm not saying this shouldn't exist. Still, they should have some comments to explain what is going on in there.
Posted by Cristian Cornea | September 11, 2007 2:22 AM
After reading through the differing opinions posted so far, I think it's pretty clear that Alberto's "First" property of crappy code is true.
However, I suspect that a number of people commenting here have too low a tolerance for crappy code. I have worked with a wide variety of crappy code at several different companies (among the crappiest I've seen included a function named "FrankFunction()"), and I've even written crappy code that I cringe at now that I have more experience.
But what I've come to learn is that the most important aspect to a company regarding crappy code is the costliness of maintaining it and adding new features.
So whether or not variable names should be prefixed, how many instructions per line is acceptable, or how long source code lines should be before wrapping are all rather minor issues relative to the big picture.
From the big picture perspective, the worst code that I have dealt with failed in these aspects:
* Little or no design before development; it is surprising how many long-term problems can be avoided just by thinking through algorithms, data structures, coupling and cohesion issues, and possible future changes.
* Documentation not maintained; sometimes projects start off on the right track, but developers make drastic changes without updating the original design documents or source code comments, or they don't bother to comment that quirky if statement they added that is necessary for that obscure special case, leaving maintainers in the dark, scratching their heads, or worse, changing something that looks broken but isn't.
* Not paying attention to colleagues' code; maintainers familiar with and working on one part of a system will make fewer errors when they start working on your part of the system if you take some time to ensure that the way you do things is consistent with the way everyone else does it *even if the way everyone else does it is crappy* (or convince everyone else that your way is an order of magnitude better)
Posted by Glen K | September 14, 2007 8:09 PM
Roger, Most error detection is wasted "crap" code. I would like to know how much code I've had to re-write to take out code which "handled" (read: "ignored") errors thus leading to illogical conditions many modules away. Errors are errors, 99% of which are application errors, i.e. programmer errors.
Posted by OldAndTired | September 20, 2007 5:10 PM
To me a code without proper documentation is crappy atleast having proper docs about what a function does and whats wrapped inside a class gives us something to help ourselves.
Posted by Pankaj Chaswal | September 21, 2007 6:15 AM
I was looking at some JavaScript code for a major e-business web application the other day, and all the variable names are like a0, a1, a2, and so on all the way up to a25 or something.
Now maybe they ran it through a code obfuscater to try and protect their intellectual property, but if they wrote it that way, I'm very afraid...
Some other bad code I've seen put about 40 or 50 statements inside a single try/catch block, meaning that it just turned any kind of error that might get thrown in there into a generic log message that basically said "something went wrong" - but who knows where.
Posted by Ryan | September 24, 2007 6:31 PM
To expand on the "error checking everywhere" point: Bad code has no sense of contracts or invariants. As a result, you never know what should always be true so you check over and over that a particular variable isn't null, etc.
Posted by Eric Smith | September 25, 2007 4:57 PM
I have worked in computers since the times of Fortran and Cobol, move to micro-computer languages before the IBM PC was ever invented and used just about any programming language that ever existed. Crappy code: I am seen.
Regarding documentation, what really ticks me off and when it states the obvious. Like: "Incrementing the offset by a farcor of m_var". Whoever wrote that must have been thinking that line of code was particularly complex, but I can read code and I can tell what it does. What I need to know is why, such as: "Offset moved to start of next line".
As for unnecessary lines in code, including script, can anyone count the lines in these 2 example and let me know which is the shorter, because it seems a lot a persons don't count as I do:
if (condition)
{
doThis();
}
else
{
doThat();
}
if (condition) {
doThis():
}
else {
doThat();
}
As far as size of functions, I think it all depends. If for whatever reason you have a method that must contain a large (huge even?) case (select in some languages) then it would not make sense to break the case down into multiple functions. In some cases arrays and loops could replace the large case, but in some cases the language does not provide such easy fixes. Therefore, what should define the size of a function is not so much how many lines it has, but how many level of indentation you need. I would say that anything above 6 levels should be defered to another function. Levels can climb up pretty fast, 2 embedded "if", 2 loops, one on the outer level and one inner, and 2 more "if/case" within the inner loop and your at the max already. A few if/else with inner if in the else will mount up pretty fast. That number of indentation should be reduced even more if repeated levels of indentation are repeated far apart of each other.
But what really annoys me the most is when managers starts defining how we should program. We had one once who required that there could be nothing within an if or a loop except a call to a function (with possibly exception for loop incrementation). Can you imagine my "if" example above being not only the norm, but a company requirement. That lead to some real spagetti code. So, managers, let the programmers do their job and have seasoned programmer do code reviews instead. Let them set reasonable guidelines and don't forget that if the code is way so good, way so clear, they we are way so expendable...
Posted by Gerard | October 8, 2007 5:40 PM
On the current monster that I maintain, here are some gems I've run across:
(*) on conditions that "cannot occur": I've seen a "style guide" from one company that had as a check-list item "include smoking gun" -- when I checked into it, it meant including plausible-looking code that never executes, the idea being that if the source code "leaked", they could point to this block of code in "someone else's source code" as proof it was stolen. Interesting concept (and presumably they filed the resulting OBJECT code as well, so they could look for the pattern in a suspected binary file)
Posted by Tom | October 11, 2007 1:34 PM
The main problem that generates crappy code is the massive copy and paste.
In day 1 you have 100 lines of code, in day 2 you have 200, in day 3 you have 400 hundred and it keeps duplicating day after day.
If on day one you had 5 bugs, in a month each bug has been converted into 2^20 identical bugs. Then removing a bug is too expensive, and you should consider that new bugs are added all the time and copied around.
Fixing a bug introduces a new one. Therefore not using unit tests and regression tests is suicidal.
Posted by BIll Black | October 12, 2007 6:44 PM