Why unused variables is such an issue?

Question

I was cleaning unused variables warnings one day, and I started to ponder, what exactly is the problem about them?

In fact, some of them even help in debugging (e.g. inspect exception details, or check the return-value before returned).

I couldn't find real actual risk in having them..

Examples

I do not mean lines that takes time of other programmers' attention, such as:

int abc = 7;

That is an obvious redundancy and distraction. I mean stuff like:

try {
    SomeMethod();
} catch (SomeException e) {
    // here e is unused, but during debug we can inspect exception details
    DoSomethingAboutIt();
}

Answer 1

The biggest issue is clarity. If your code has a bunch of extraneous junk in it, and I am maintaining your code, I have to figure out what everything does.

Is that variable ever used for anything? Perhaps you do perform operations on it, but never use its value for something. Simply incrementing a variable doesn't mean the variable serves a purpose if you never read it (return, input into another expression, et al).

However: if a variable has a purpose and is named appropriately it does not detract from the code as a whole.

Given the examples in your updated question and some others I thought of:

• Having a function that calls another function, stores the result, logs it, then returns it is a pretty short and clear.

• Splitting a statement with multiple expressions into multiple statements (e.g. splitting a long, complex calculation into multiple statements with multiple intermediate variables) can make the code more clear despite being more verbose. With less information to process at a time, the brain can more easily grok what is going on.

• Not using exceptions is perfectly fine: most languages require specifying the exception being caught including providing a variable name to be used in the catch block. There is no way around this in many popular languages and programmers are used to it.

If I need to spend time figuring out what code elements have a purpose and which are only wasting my time, my productivity decreases. However, if the code is fairly concise and variables are well-named even when they appear to be extraneous at first, this mitigates the risk of wasting unnecessary time reviewing the code to understand it.

Bad code with extraneous variables, empty blocks, and dead code is one way developers get a bad reputation around the office: "I was fixing a bug in Snowman's code. I spent two hours figuring out what that function was doing, and thirty seconds fixing the bug! Snowman is terrible at programming!" But if the code serves a purpose and is written in a clear manner, it is perfectly fine.

Answer 2

According to the frame you have given, it is hard to argue against those variables per se:

try {
    SomeMethod();
} catch (SomeException e) {
// here e is unused, but during debug we can inspect exception details
DoSomethingAboutIt();
}

Take this for example. You are catching an exception and (perhaps) have code which deals with the exception in a proper way. The fact that you aren't using the actual instance of the exception does no harm, neither to the code flow nor to the understandability.

But what makes me think is when you write about the legitimation of »unused variables«

In fact, some of them even help in debugging

That is not the purpose of writing code: having to debug it later. In order to prevent misunderstandings, I have to say, that I am well aware, that oftentimes you have to fix your code; and sometimes debugging is the solution. But it should not be the first, what comes to your mind, when writing code, that you leave "anchor points", whose only purpose it is to make debugging easier.

SomeClass Func() {
    // ...stuff...
    var ret = FinalComputationResult(thing, foo, bar);
    // ret is unused, but while debugging allows checking return value.
    return ret;
}

Here it depends how complex stuff is and how that is related to the value, which is returned.

As a "Golden Rule", I would say the following:

If it helps understanding the purpose of the code, it is okay to incease redundancy in your code

or to put it otherwise:

Write your code as redundancy free as is necessary to understand later easily, what the code is doing

That results in: Most of the time, you should remove "debug"-variables.

Answer 3

Unused variables that serve no obvious purpose are a code smell in my view. At best, they can be a distraction - they add to the general noise in the module.

The worst case is that subsequent coders spend time figuring out what it was supposed to do and wondering if the code is complete. Do yourself (and everyone else) a favour and get rid.

Answer 4

Unused variables make the intent of your code unclear. This is bad because despite appearances, code is predominantly written for people to read, not for computers.

Others have already pointed out that constructing a value and not using it confuses other people who have to read and work with your code. However, in my view the greater danger is to yourself.

An unused variable might be intentional, or it might be an oversight pointing to a defect. For instance, you might have mistyped a name and stored a value in one place when you thought you'd stored it in another. The resulting program could run fine but silently give the wrong result.

Analysing data flow can help you find such errors and many others. Therefore it pays to write your code in such a way that everything a data flow analyser points out as an anomaly is, in fact, a bug. Automatic assistance in preventing bugs is invaluable; many people think "Oh, I don't need assistance, I would never be that careless", but so far everyone I've met who thought that was wrong.

Answer 5

There is a coding style that involves deliberately assigning anything that was (or might become) of interest to a variable for the specific purpose of easily looking at it in a debugger. This is especially useful for code which looks like:

return foo(bar(), baz(wot(12), wombat(&badger)));

Even with somewhat better names for the functions, it can be easier to work out what's going wrong in the call to foo when written as:

auto awombat = wombat(&badger);
auto awot = wot(12);
auto abaz = baz(awot, abadger);
auto abar = bar();
auto afoo = foo(abar, abaz);
return afoo;

I'm yet to work with people who practice this as the default configuration, but rewriting a piece of code which is stubbornly refusing to make any sense in single assignment form can make it easier to work out what's going on under the debugger. We always reverted the code back to compact, "pretty" format afterwards but I wonder whether that was a mistake.

The alternative is walking up and down the stack and hoping you don't step too far, or using (the distressingly rare) reversible debuggers.

It's not a popular strategy, but as it's helped me work through some absurd bugs I don't think it should be written off as intrinsically bad.

Answer 6

For people who include their unit test code in the same projects or modules as their "real" code, an unused field, like an object method, is a strong signal that your unit tests aren't testing everything. This increases the chance that a developer will try to use that currently unused field or method, and run into a bug that wasn't being caught until then.

An unused variable in a somewhat nontrivial subroutine or method may indicate that the programmer meant to use it, but is using another variable by mistake (say, after cleaning up after a copy-paste operation).

An unused variable in a more trivial subroutine is probably either unnecessary dross, as other people have answered. Occasionally, it's used in a breadcrumb statement that's currently commented out:

SomeObject tmp = someMethod();
// printDebugInfoAbout (tmp);

...it's clear to the eyeball that it's used there, and only there (i.e. someMethod has an important side effect), but the IDE doesn't know that, and removing that variable is more trouble than it's worth. I tentatively use warning suppressors in such cases.

Answer 7

It is easy when you write code to mix up two variables. You compute a variable B but then use instead some variable A created earlier.

So a possible reason for an unused variable is that there is a mistake in your code. That is why the compiler warns you. If you "don't do" unused variables, it is even a dead giveaway.

When you see such a warning, you should verify your code and either fix it or remove the offending variable. If you don't remove it, you will stop paying attention to these warnings, and you might just as well disable them completely. But then you will eventually loose an afternoon debugging the code because of one silly mistake.