33

I've heard stories of this from senior coders and I've seen some of it myself. It seems that there are more than a few instances of programmers writing pointless code. I will see things like:

  • Method or function calls that do nothing of value.
  • Redundant checks done in a separate class file, object or method.
  • if statements that always evaluate to true.
  • Threads that spin off and do nothing of note.

Just to name a few. I've been told that this is because programmers want to intentionally make the code confusing to raise their own worth to the organization or make sure of repeat business in the case of contractual or outsourced work.

My question is. Has anyone else seen code like this? What was your conclusion was to why that code was there?

If anyone has written code like this, can you share why?

3

14 Answers 14

16

I have heard developers who try to make their coding achievements sound more complex than they really are. I've never heard anyone admit this, but I have seen code that meets your criteria that was created intentionally out of haste or poor practices and not sabotage. The code surrounding the maligned code may have been altered to the point where a particular function is no longer useful.

Someone would actually have to see this code first-hand before coming to the conclusion that only this developer can manage the complexity. Most managers and other business people just come to this conclusion because they don't understand any kind of code and don't want to refill the position.

4
  • 2
    I'm inclined to give you the correct answer in this case because some of the code I see simply can't be unintentional... not unless someone was high when they coded and thought it would just be funny! I do believe others have relevant reasons for useless code as well, but the code I see are on projects that a few people have worked on and I'm the first guy outside of the original development team working on it. I have to say that it seems like a case of complexity added to shock and awe.
    – Ali
    Aug 21, 2011 at 21:45
  • 18
    @Ali: Never attribute to malice what is better explained by incompetence. In other words - the code probably evolved to this kind of mess because nobody was brave enough to spend the time to actually look at it and see what it actually does. This all sounds like a bunch of quick fixes applied, over and over, until all thats left is a bunch of yuck. Aug 22, 2011 at 6:26
  • 1
    +1 for @quickly_now. That's usually what ends up happening; everyone is afraid to touch anything that "works" for fear of breaking it (or, Heaven forbid, taking longer on a task to actually improve the code! The horror!). So the code rots and festers and finally collapses many years down the road. Aug 22, 2011 at 12:58
  • @Ali, there have been cases when code that seems the most semantic and reasonable has been described as me probably thinking this is funny or showing off. And vice versa with me seeing some other people's code. You never know who's crazy, and most of the time it just comes down to experience and preferences. (not talking about objectively bad code here, just that such descriptions are easily thrown around) Jan 30, 2019 at 21:45
73

I haven't see the code like this but I have seen the code that looks pointless or is pointless for the other reasons:

  1. Backward compatibility. You found much better way to do things but you must keep old (and by now not very useful) API/function because some third-party module out there may be using this API/function for something. Even if the function doesn't do anything useful, absence of it might break some code.

  2. Defensive coding. You know the checks in this code are pointless because this was already checked elsewhere. But what if somebody changes this elsewhere code and removes or changes the checks so that they won't longer match your preconditions?

  3. Organic growth. In big projects, over the years many things change, and it turns out some methods that were used before aren't used anymore, but nobody bothered to remove them since nobody kept track of if this specific method is used or not, they just refactored their pieces of code and by chance it happened they all stopped to use this method. Or conditions that once had meaning but application was refactored in other places so that condition became always true but nobody bothered to remove it.

  4. Over-designing. People might code some things "just in case we'd need it" and never actually need it. Like "let's spawn a thread in case we'd have to do some work offline" and then nobody asks to do anything offline and the programmer forgets about it and moves on to other projects (or maybe even another company) and that code remains there forever because nobody knows why it's there or if it's safe to remove it.

So while I have never seen it done out of malice or misguided approach to job security, I've seen tons of times when it happens as natural result of software development.

3
  • 22
    I think #3, Organic Growth explains a large fraction of the Useless Code I've seen on the job. But all 4 of these reasons assume an intelligent programmer. Some useless code derives from someone not understanding what needs to happen, and what doesn't, and leaving in a lot of code purely out of fear of changing what (sort of) works. Aug 21, 2011 at 1:39
  • 2
    I have seen #4 in my project: often it is not done on purpose to have more power inside the company, rather there are people who always try to create a more general solution, even if it is not needed. Regarding #2, I use it myself a lot for exactly the reasons you explained: IMHO even the smallest function or method should not make any assumptions as to how the rest of the code works or will change. Instead, my code follows the simple pattern: "if input OK then output else error". This follows the general design principle of minimizing dependencies.
    – Giorgio
    Aug 21, 2011 at 10:03
  • 3
    You also forgot: bad developers. Some people who are writing code, should not be, and good review processes don't exist in many shops.
    – Joe
    Aug 22, 2011 at 1:18
20

My question is. Has anyone else seen code like this? What was your conclusion was to why that code was there?

1) Yes.

2) In the cases I've seen, I'd put it down variously to:

  • Programmer inexperience
  • Programmer not understanding a particularly complicated and/or poorly executed design he is attempting to modify
  • Programmer being interrupted in the middle of (say) a refactor.
  • Carelessness

Now maybe I'm being charitable about it, but my general approach is that it is better to be forgiving / non-confrontational about these things, than to point fingers and carry on about poor quality. Obviously, things could get bad enough that something has to be done, but a gentle nudge in the right direction is usually sufficient.

Of course, you can't take such a laissez-faire approach if quality / mistakes are going to seriously impact "the business". But in that situation you need mandatory and diligent code reviews of everything, combined with a comprehensive test procedure.


In my experience, people tend to get "up tight" about poor quality code (at least in part) because it offends their personal standards. It is all very well to (personally) strive for perfection, but it is a bit unreasonable to project your personal standards onto other people. By the sounds of things (e.g. from the nature of your examples), this is what you are doing.

IMO, this is not productive, and not conducive to a good working relationship with your coworkers.

2
  • +1 Was typing a response and found you pretty much listed all the reasons I was going to mention. Aug 21, 2011 at 4:39
  • +1 for being charitable. Fix someone's mistakes without pointing fingers and your colleagues will respect both your technical skills and your people skills. Harangue the author for their crappy code and your colleagues will resent your approach and discount your abilities.
    – Caleb
    Aug 22, 2011 at 14:42
13

All of those are often symptoms of how a project ages.

 1. Method or function calls that do nothing of value. There are plenty of times when some code is simply left as is (hopefully with a big deprecated warning, but since most languages don't have that warning, it isn't always followed...) because, at one point it served some genuine purpose and no one knew what might happen if the offending lines were removed.

I seem to remember this from a dailywtf:

@deprecated // he might have been crazy enough to use reflection...
boolean getTrue() {
    return false; 
}

 2. Redundant checks done in a separate class file, object or method. Layers of communication are also imperfect (ever read Mythical Man Month? If not, what are you doing on the computer!? Go! READ!). Often, one person will work on something and then leave the project, and then the next guy, finding some bizarre bug, throws an extra check here and there to try to eliminate it. When the bug is removed, the checks aren't because, well, if it isn't broke, don't fix it.

 3. if statements that always evaluate to true. Oh, I've done this one. I got a project once, it had a series of probably 10-15 if/else blocks. To change the behavior, I simply put a true|| at the first block. It wasn't until months (years?) later that I came back and said, "Oh, wow, this code was supposed to have been destroyed but never was"

 4. Threads that spin off and do nothing of note. I can imagine a line of thought going like this:

  1. I know! I can handle these two problems asyncronously! I'll create threads foo and bar.
  2. (two months later) Huh, you know, the functionality from bar is a little bit better in foo. I'll move some over.
  3. (one year later) You know, putting this other stuff from bar into foo makes sense.
  4. (many, many years later) "Hey, this bar thread doesn't look like it is doing anything, can we remove it?" "Better not, it's been there many, many years..."
1
  • 5
    +1 for "Better not, it's been there many, many years..." - this happens over and over. Fear of removing because of fear of the consequences ("How do we test that we did not break something" - especially if there are no unit tests around). Aug 22, 2011 at 6:29
11

I'm a bit more of an optimist. I think what you have descried often occurs when code is refactored carelessly.

1
  • 13
    Although it's hard, Never attribute to Malice what can be explained by Stupidity. Aug 21, 2011 at 1:36
8

Old fellows told me of a time when consultants where paid by the number of lines of code they produced. And so they maximized profits using amazingly long-winded constructs.

Nowadays I always assume the guy is still learning the language while doing the work. And he's in a hurry.

1
  • Talk about cutting off your nose to spite your face. I guess it's ok if you never have to look at the code again.
    – JeffO
    Aug 22, 2011 at 18:35
6

Most answers boil down to these two simple facts:
[1] Code reflects the history of the code, and
[2] Code reflects the expected future of the code.

I have written functions that do nothing of value, IN THE CURRENT APPLICATION ENVIRONMENT given THE CURRENT SPECIFICATIONS, but may be needed in the future.

I have written if-statements that, AT PRESENT, always evaluate to true. But maybe in the past it could be false.

As to redundant checks, hey, I don't trust other code, I don't even trust my own code. If a module depends on N being 1 or 2 or 3 it darned well better make sure of that, and crash informatively if it isn't. It's illegitmate for Module B to explode because Module A screwed up; it is quite legitimate for Module B to complain that Module A screwed up. And remember that, next month, that parameter may be coming from the as-yet-unwritten Module C.

2
  • 1
    I call that bad coding. You expect that you'll need it in the future, but that rarely happens. YAGNI. Writing an if that always evaluates to true is wasted effort and confused the person that must add quite likely different functionality to begin with. That parameter that's coming next month can wait until next month to be added. Cluttering code NOW is pointless.
    – Andy
    Aug 21, 2011 at 19:23
  • 1
    if ( language='en' or language=th' ) - maybe next month we try Chinese? if ( ! isset( $TITLE ) ) - all modules are SUPPOSED to set $TITLE, but maybe somebody someday spells it wrong. if ( file_exists( $TARGET ) ) - good code will have created the file already, but maybe there's a system error and it didn't get created. My standard PHP / MySQL interface code always checks for error, even though I've never caught one yet. Sep 30, 2011 at 8:48
3

I've seen this a few times, in fact just yesterday, I've got to merge some of my bosses code into my new app. In his case it is just down to a general lack of skill and understanding and the belief that he thinks he is quite a skilled developer.

'Method or function calls that do nothing of value.' and 'if statements that always evaluate to true' are a major issue with his code.

3

I suspect that although many have seen code that has these issues, few would fess up to writing the same. In all likelihood, what you're seeing is accumulated software rot - someone adds something, which doesn't really work, the next maintainer adds protective code further along in the chain to guard against the condition that wasn't properly checked in the first place; then someone gets a problem report and adds even more armoring against a specific instance of a problem; another person adds a more general check and forgets to remove some of the old code added previously which dealt with more specific symptoms, etc.

Then there's the problem of code cleanup: there's no particular incentive to remove what appears to be dead code, and tremendous incentive not to do it, because if you don't understand the code completely, your assessment that the code is "dead" will be flawed, and you will wind up breaking the system.

2
  • Method or function calls that do nothing of value.

Not necessarily bad. Methods in a base class often call empty methods that are meant as override points for subclasses. Example: Cocoa Touch's UIView has a -didAddSubview: method that's documented as doing nothing in the default version. UIView's -addSubview: method has to call -didAddSubview: even though it does nothing because subclasses may implement it to do something. Methods that do nothing and the reasons for them should be documented, of course.

If an empty or useless function/method is obviously there for reasons historical, it should be excised. Take a look at earlier versions of the code in your source code repository if you're not sure.

  • Redundant checks done in a separate class file, object or method.

Hard to say if that's okay without some context. If the checks are clearly done for the same reason, it may mean that there's not a clear separation of responsibilities and some refactoring is called for, especially when both checks result in the same action being taken. If the action resulting from the both checks is not the same, then the two checks probably are being done for different reasons even if the condition is the same, and that's likely okay.

  • if statements that always evaluate to true.

There's a big difference between:

if (1) {
    // ...
}

and:

if (foo() == true) {
    // ...
}

where foo() happens to always return true.

The first case happens a lot when people are debugging. It's easy to use an if (0) {... to temporarily remove a chunk of code while you're trying to isolate a bug, and then change the 0 to 1 to restore that code. The if should be removed once you're done, of course, but it's easy to forget that step, or to miss one or two if you've done it in several places. (It's a good idea to identify such conditionals with a comment that you can later search for.) The only harm is the confusion it might cause in the future; if the compiler can determine the value of the condition at compile time, it'll remove it entirely.

The second case can be acceptable. If the condition represented by foo() needs to be tested from several places in the code, factoring it out into a separate function or method is often the right thing to do even if foo() always happens to be true right now. If it's conceivable that foo() might eventually return false, then isolating that condition in a method or function is one way to identify all the places where the code relies on that condition. However, doing that does create some risk that the foo() == false condition will go untested and could lead to problems later; the solution is to make sure that you add unit tests that explicitly test the false case.

  • Threads that spin off and do nothing of note.

This sounds like an artifact of history, and something that could be identified either during a code review or through periodic profiling of the software. I suppose it could be created intentionally, but I have a hard time imagining that someone would actually do that on purpose.

1

It happens. Quite often actually.

Sometimes these coding dead-ends are more like old goat-trails which have fallen into disrepair when a more efficient/modern/speedy freeway has been installed around them.

On other occasions (and I am probably guilty of this), they are foundations for extension of the software when a briefed, but unconfirmed set of features/functions are requested. (Sometimes putting in a bit of work in the initial build providing handles and the like for later work which you plan to "bolt-on" can make life easier, when it happens, or more difficult/messy if the further work doesn't eventuate.)

And, alot of it is directly due to the old "If it ain't broke, don't fit it." Sometimes tearing out code you do not understand, or believe is unused, can cause the programming version of "The Butterfly Effect". Have had that happen once or twice too.

0
1

Sometimes I'll have a global boolean set to true, and later in my code an if( bool ), then during runtime, I might set a breakpoint at the if statement, and switch the boolean to test something.

0

I object to if true statements to be indiscriminately classified as "pointless code".

There is a legitimate case for using an if (1) { ... } block in C code that either wants to be compatible with the C standard that insisted on variable definitions to be at the start of a function, or just wants local variables to be defined as locally as possible.

switch (i) {
    case 23:
        if (1) {
            /* I can declare a local var here! */
        }
        break;
}
2
  • 5
    There is no need for the 'if (1)', why not just have the block?
    – FigBug
    Aug 22, 2011 at 6:35
  • 3
    Both C/C++ and C#, and I'm pretty sure Java (as well as likely many other similar languages) allow anonymous statement blocks; no need for an if, while or similar construct. It is unlikely to be very clean, but it certainly is allowed according to the language spec.
    – user
    Aug 22, 2011 at 9:21
0

A professor of mine related a story to us one day that a previous employer would pay them based on the number of lines that they completed. So, they wrote several multi-dozen lined functions that were never called. Seems like a great reason to write pointless code.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.