8

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();
}
  • 27
    You mean other than the fact that the programmer coming after you is going to have to spend time trying to figure out why they are there? – Robert Harvey Jan 18 '17 at 15:59
  • 1
  • 1
    Your examples are harmless, but I've never seen cases like that referred to as "unused variables". Where are your warnings coming from? – Jacob Raihle Jan 18 '17 at 16:19
  • 9
    Well, ret is not unused. – Robert Harvey Jan 18 '17 at 16:22
  • 4
    You wrote "I mean stuff like catch (SomeException e)" - you are pretending there are other, similar cases. Enlighten me, which ones? I ask because in C# or C++, I have trouble to find a different situation for your case. – Doc Brown Jan 18 '17 at 21:23
3

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.

19

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.

  • 9
    +1 for "Snowman is terrible at programming." I always suspected. How can you be a good programmer when your fingers are always melting? – Robert Harvey Jan 18 '17 at 16:18
  • See my edit - I don't mean mere variables created out of the blue for no purpose, just those that may assist here and there during debug, or just generated by one of those automatic tools like catch blocks. – Tar Jan 18 '17 at 16:22
  • 1
    @Tar that changes the entire question - edit incoming. – user22815 Jan 18 '17 at 16:24
  • Note that in your bulleted list of when you do this, the first two tend to not actually result in warning (I've never seen a language that produced a warning in such a case). For the third case, any language that requires the exception be given an identifier doesn't have a warning for it, and any that produce a warning for it don't require it to have an identifier (again, that I'm familiar with anyway). – Servy Jan 18 '17 at 18:06
  • 2
    Looking at the revision history I don't see any indication of the question fundamentally changing. As far as the OP's code goes, the first two will result in a warning in C# (which looks to be the language used). The third example would not result in a warning. The comments on the question also indicate that they're specifically indicated in compiler warnings, and when Robert pointed out that the third example doesn't produce a warning, the author conceded that it was a bad example for their question. – Servy Jan 18 '17 at 19:17
5

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.

  • 1
    What is a "bug" in your view? Unused variables do not produce unexpected, erroneous outcomes per se, which is what most people would qualify as a bug. – Harris Jan 18 '17 at 16:14
  • 5
    Why is the variable unused? Very likely because it should have been used, but another variable was used instead by mistake. You get a compiler warning, and you fix it and fix the bug at the same time. Problem is if you have an undisciplined programmer who leaves a mess around, you can't find where that kind of warning indicates a bug and where it just indicates a messy programmer. – gnasher729 Jan 18 '17 at 19:38
  • @HarrisWeinstein Given testers and devs can't even agree what is a bug most of the time, I'm going to park that for now. Suffice to say it is a deficiency in the code - and possibly the executable as it will take up space. Admittedly, not the biggest problem in the world but up there with comment box spelling mistakes and commented out code. It is cruft that adds nothing so should be removed. Code that looks good inspires confidence. I think this quotation is also apt and covers the refactoring process well. – Robbie Dee Jan 18 '17 at 19:52
  • If you're using a dynamic language, assigning a value to some_var when the rest of the code uses someVar is almost guaranteed to be a bug. This might not be problem in Java or C but it can easily break some Python. – Sean McSomething Jan 18 '17 at 22:13
  • 1
    @ThomasJunk That is probably a better term for it - thank you. I shall adjust my answer accordingly. – Robbie Dee Jan 19 '17 at 10:39
3

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.

2

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.

  • While I probably wouldn't go as far as you did I find your first "pretty" form too compact. – Loren Pechtel Jan 19 '17 at 2:45
  • Except that there isn't a single unused variable in your code. – Florian F Jan 19 '17 at 21:10
  • On the positive side, if you were to get a warning about unused variables, you would know that somewhere in that transformation you committed some major blunder. Which would be easy to fix. – gnasher729 Jan 19 '17 at 21:45
1

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.

  • 1
    I'm wondering under what circumstances you'd want to have commented-out code committed to your repository. But a similar situation arises when using conditional compilation (such as when using the assert macro in C or C++). Unused variable warnings can indeed be an annoyance there and suppressing them is the most reasonable response, if you ask me. – 5gon12eder Jan 18 '17 at 20:42
  • 1
    I've edited my answer, adding a very rough example of what I've seen in the past. The block might be run many times, and something's collecting all the debug info in a log for analysis when tracing a problem, but printing is slow and so it's usually turned off. I could replace the first line with someMethod and lose info on where tmp came from. Or I could commit a clean version and forget that some past version had that useful breadcrumb (and lacked necessary code added in later versions). ...I've sought an ideal tradeoff on this for a long time now. – Paul Brinkley Jan 18 '17 at 21:01
  • I see. Apparently, your language of choice does not have a pre-processor. ;-) – 5gon12eder Jan 18 '17 at 21:08
  • Currently, aye. :-) Although to be fair, I've seen this same sort of stuff in C and C++, and it likewise wasn't easy to get rid of unused variables in those cases, either (quasi-appendices?). ...The fact that my last big C/C++ project was done at a time when IDEs didn't exist (compiler warnings required running "gcc -pedantic") also didn't help. – Paul Brinkley Jan 18 '17 at 21:18
0

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.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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