Pointless Code In Your Source

Question

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?

Answer 1

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.

Answer 2

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.

Answer 3

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.

Answer 4

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..."

Answer 5

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

Answer 6

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.

Answer 7

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.

Answer 8

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.

Answer 9

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.

Answer 10

• 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.

Answer 11

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.

Answer 12

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.

Answer 13

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;
}

Answer 14

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.