Hacker News Reader

Comment by yjftsjthsd-h | Hacker News Reader

> Eventually it clicked for me: developers don’t intend to write useless code, and code that does not match the developer’s intent is by definition a bug. Therefore, any useless code you can detect is a bug.

I'm not completely sure I would take it all the way to calling it a bug, but I do appreciate a rigorous way to simplify code, because the worst case is that you've made it easier to reason about and that's a win all by itself.

(EDIT: This post also makes me feel better about my personal coding style being paranoid and doing things like using parens to force order of operations and avoiding "advanced" constructs like ?? because I don't trust myself to not shoot myself in the foot. I'm not a professional dev, so I'm happy to write verbose, inelegant code in exchange for it being so simple that I'm less likely to screw it up)

View on HN

I am a professional developer, and I'll take the verbose, ugly, but readable code every time. Clever code that makes me work harder to understand what it does has a greater chance of being missed, blindly trusted, or just misunderstood, and that causes us untold numbers of headaches.

Writing clear, concise code is *hard.* Writing just clear code, conciseness be damned, is a perfectly fine middle ground.

OK but if you have large amounts of unused code throughout the codebase, mistakes are more likely to be "missed, blindly trusted, or just misunderstood".

> but if you have large amounts of unused code throughout the codebase

Delete it? Version control is your backup

You have to find it, first. I have a PHP codebase where symfony and doctrine are using strings to connect distant parts of the code. Just trying to find what calls what is hard. I am in the process of replacing as much strings with real calls and adding as much typing as possible, and have thrown away 1/3, but the code base fights back.

I think when the OP says concise they mean using several lines to make some logic clear, rather than a tricky little one liners. (like many of the mistakes in the article)

One example of this I HATE is ruby’s obsession with

return X unless [complex logic]

I sometimes see this stacked inside other complex logic and it’s a nightmare sometimes to figure out the actual control flow.

Which desugars to: if (!complex logic) return X;

If you put OR between two ANDs without a paren I think you have bigger issues than unused code.

I agree. (!someBool == anotherBool) very bad code if you ask me.

yeah, it's reassuring that for most (all?) of these I'm like "I would never write that"... though I also lean towards very "simple" code that is not only blatantly clear but totally non-"clever". I'd rather the early-in-career dev we bring in 6-12mo later can read it and immediately parse it. Plus it's easier for ME 6-12mo later! hahah :)

agreeWithPreviousCommentOnUselessCode()

I'm also a professional dev and I 100% agree with you. Clever code is a trap. Clever coders… Well, pretty much a disaster IME. KISS

Author of the rule and post here. I reread the post this morning, and I agree that I should have been less definitive in that sentence. But I stand by the broader point: Useless code is generally not something developers intend to write. When we do, it's generally something exceptional, so a lint suppression is a reasonable way to clarify "I meant to do that". And in the common case where it was an error, the lint rule proves quite helpful.

I hope the take away for the reader is: If you can think of other rules that will detect useless code, you should pursue them, because they are likely more valuable than just enabling dead code elimination. They have a high probability of being able to uncover interesting bugs/mistakes as well, which is much more valuable.

I often add an extra line or two of no-op code in order to improve readability. But in these examples, the code clearly wasn't written to communicate anything, but rather by mistake.

Rule author here. Would love to see some examples! I think the closest I’ve (knowingly) done to this is to add an empty else clause that contains a comment.

So one thing I try to be very good about is never doing more than one thing in a line, which means always using named conditions and named returns. So in other words, if statements and return statements should always contain a single variable that describes the condition or what's being returned, and should not contain function calls, boolean logic, etc. This typically involves giving things descriptive names and then not using them more than once, but it makes the code more readable.

So e.g. you have lines like:

  const isCar = (wheels === 4) && (steeringWheels === 1) && (canDrive === true);
  if (isCar) { ... }

and not lines like:

  if ((wheels === 4) && (steeringWheels === 1) && canDrive === true) { ... }

And similarly:

  const isCar = (wheels === 4) && (steeringWheels === 1) && (canDrive === true);
  return isCar;

Rather than:

  return (wheels === 4) && (steeringWheels === 1) && (canDrive === true);

I'd actually like linters for JS and Python to enforce named conditionals and named returns, but afaik no one has done this yet.

I don't think the author was looking for this kind of example. Everything in your code is useful, so this category of linter would not complain.

Other linters might complain about `==`, though ;)

Interesting. I see what you're saying, and I don't know if I would characterize that as no-op code. All of that code will run. What I see there is redundant assignments, which I agree can act as a great form of code comment.

Often a code comment can be avoided (and clarity improved) by giving a name to an intermediate value rather than letting it be a nameless expression.

I like it! But I don't think of this a no-op code and would not imagine a lint rule objecting to it. That said, a code minifier (or compiler) would be well positioned to optimize that code, so you shouldn't even have to worry about even the thought of perf implications.

I wouldn't call it a bug either.

I quite often write 'useless' code, just to make the intent more obvious.

Int Foo,bar = 1,5

Int baz = foo+bar

I could just make baz 6, but then I don't know why it's 6.

Your code executes and is not useless.

> The rule checks for comparisons (==, !==, etc) where the outcome cannot vary at runtime, and logical expressions (&&, ??, ||) which will either always or never short-circuit.

If you write if(baz == 6) then the if clause is useless and probably a bug.

A general rule was extracted that 'useless' code is buggy code

I'd expect the compiler to optimise out

If(somedebugflag)

Not tell me off for writing buggy code.

Reply on news.ycombinator.com