Hacker News Reader

Comment by dietr1ch | Hacker News Reader

I think that these bundled conditions are bad, and that having negative booleans is terrible. Using and almost abusing early returns on bad input almost always leads to easier to follow code.

    if (someLongNamedVarIsNotUnknown && someLongNamedMapCountIsZero)
        return false;
    else
        return true;

This is not the same, but I'd suspect there might be a bug in the code above

    if (someLongNamedVarIsUnknown) {
      return false;  // Can't do much
    }
    
    if (someLongNamedMapCountIsZero) {
      return false;  // Nothing to do
    }
    
    // Do stuff
    return true

The named booleans suggested here help adding clarity, but more often than not they'll end up being single-use (const) variables that just paraphrase their definitions. In these cases I'd rather avoid introducing a variable, and rely in comments if I really need to explain something.

View on HN

> The named booleans suggested here help adding clarity, but more often than not they'll end up being single-use (const) variables that just paraphrase their definitions. In these cases I'd rather avoid introducing a variable, and rely in comments if I really need to explain something.

Interesting. With the same axioms I come to the opposite conclusion. That comments are to be avoided except when strictly necessary, and giving names to concepts via intermediate const variables is one of the best weapons in that fight. Ie. make your code read as close to what the comment would be (but simply via naming) makes it very clear when the names no longer match the behavior, in contrast to comments which have a tendency to bit rot.

This is what I used to love about Obj-C - idiomatically written Obj-C code was always so much easier to read and understand without needing comments.

> The named booleans suggested here help adding clarity, but more often than not they'll end up being single-use (const) variables that just paraphrase their definitions.

That's ok. The worst tradeoff in the "named booleans" pattern proposed in the blog post is that it misses the whole point of chaining predicate evaluations, which is leveraging short-circuiting to prevent further predicates from being evaluated.

Chaining logical comparisons and the way they short-circuit is specially important when a predicate has preconditions and/or is expensive and/or has side-effects.

If your predicates are hard to read you don't fix that mess with "named booleans". You fix them by extracting them to a pure function like

   isNotUnknown(_someLongNamedVar) && isNotEmpty(_someLongNamedVar)

Nest the if statements then, perhaps in an else branch. Relying on short circuiting to avoid side effects or improving performance meaningfully is pretty shaky.

> Relying on short circuiting to avoid side effects or improving performance meaningfully is pretty shaky.

Says who? IME relying on short circuiting is useful and idiomatic in C and C++ code, e.g.:

    if (callback && callback->run()) { ... }

In your opinion would it be better to write:

    if (callback) {
      if (callback->run()) {
        ..
      }
    }

The situation is even more awkward if you try to avoid || short-circuiting, since you need to duplicate the body of the conditional.

I tend to agree with toxic here. Side effects inside conditionals should be avoided, in short circuit conditionals even more so.

In a case like this, my preference would be to give callback a dummy value and always run it.

I subscribe pretty heavily to the Carmack philosophy of always running the code as similarity as possible and making a decision at the end of a procedure.

> in short circuit conditionals even more so.

I'm curious, what leads you to believe that skipping unnecessary calls to functions with side effects is something to be avoided? Do you believe that unconditionally calling code with side effects specially when you don't have to is something to be desired?

I believe that running the same code path as much as possible per software cycle is to be desired. The code I'd call in the case that client code didn't specify a function wouldn't produce any side effects. It would be an empty function object/pointer.

I would almost always rather support default behavior through dummy data rather than null checks. I find that a lot of software bugs come from the unintended consequences of branching. Especially branches which create state that is consumed later.

In fact, I find that most null checks lead to bad software behavior as it often hides bugs. Early outting when null is detected is especially silly to me. The client has just tried to run a procedure which had to be abandoned due to a lack of data and you're just going to quit silently?

If you're using dummy data for default scenarios and asserting not null rather than checking, you will eliminate most logical null checks in your code. Though I do concede that in the case where you must support null checks, such a short circuit as above is acceptable and in fact preferable to me over two if statements.

I think side effect is a red-herring here: it's idiomatic to rely on short circuiting when the RHS relies on the LHS being true to be safe to call at all.

In the above example, we check that the pointer is non-null before calling it. The "side effect" we are avoiding is a segfault.

In the code that splits the conditional into two, the callback is still run inside a conditional, just by itself.

> In your opinion would it be better to write:

in mine, yes definitely 100% the second code is more readable than the first

> in mine, yes definitely 100% the second code is more readable than the first

If anything, adding a nesting level for each chained predicate makes code harder to read, and needlessly so.

It makes code a tiny bit more verbose but much more readible and most of all easily extendible without having to worry about bracket positions

> It makes code a tiny bit more verbose (...)

The issue is not verbosity. It's the needless addition of nesting levels and independent expressions in a non-idiomatic way, which greatly increase the cognitive load of checking a single row of chained predicates. It's bad code.

for me the cognitive load of

    if(an expression) {
      if(another expression) {
      }
    }

is MUCH lower than

    if(an expression && another expression) {
    }

which always raise the questions of:

- did the author think about operator precedence

- did the author want to use && or &

- did the author think about short-circuiting (as evidenced in this very thread, a lot of people do not)

And not to forget, is it okay to omit parentheses, does && take precedence over any other symbol in the expression?

I subscribe to the idea that your code should state your intent as closely as possible.

I question the idea that you would ever want the very linchpin of your function hidden away as a second subclause of an if statement. A function that calls callbacks is probably mainly doing that, let it stand proud and clear in the code text.

If your function is calling callbacks left and right “on its way somewhere else”, I think a refactor is in order.

Maybe you will now present some second scenario with something other than callbacks. Maybe in that case you’ll have me convinced it’s an okay exception. For you see, I said only it’s “pretty shaky”, not a mortal sin.

With that said though, I think in 9/10 cases, it’s something you can solve with an early return.

    if (!callback)
        return;

    callback();

> Relying on short circuiting to avoid side effects or improving performance meaningfully is pretty shaky.

You've got it all backwards. Short-circuiting is not used to avoid side effects. Short-circuiting is used to avoid needless calls to any predicate, which can have and often do has side-effects, and if it doesn't have today it might have tomorrow.

If you don't call them when you don't have to, your code is in a better shape.

And let's not pretend that unconditionally macode with side effects when you don't have to is a good practice.

I usually prefer to write more readable code, and then optimize later if necessary. If the short circuit becomes critical for performance then I would leave a comment, or as the parent suggests just order it with an early return.

> If the short circuit becomes critical for performance the

The point is not performance. The point is readability, and expressing things clearly and with the appropriate level of detail and leaving out nesting and scope. Safety and performance are important but secondary advantages.

True, functions with return values are so much better in such a case. I really miss if expressions and switch expressions in most languages. Most functional languages (and also c#) have them. Something like (let’s do JavaScript in this example):

  const value = if (a) {
    return true
  } else {
    return switch (b) {
      case 1: return false
      default: return undefined
    }
  }

Interestingly Rust does this and I agree JavaScript with const feels only 80% there without this feature.

As an aside for those wondering what the name for this is to see what other languages support it, this is a case 'if' being an expression rather than a statement which allows it to be composed within other expressions.

A predicate?

No, if, and match, and most things are expressions, and thus have a (typed) value.

In a typical programming language we can write something like x = 5; assigning 5 to the variable x. That 5 there is an expression, we can write x = (3 * 2) - 1; with the same effect, that's a more complicated expression. We can call a function here: x = f(3); or if our language has methods we can even call those: x = some.function(parameters);

In Rust I can write x = if it.should_be_nine() { 9 } else { 5 }; // because if is an expression

Since Rust's loops are also expressions, x = loop { let stuff = get_stuff(); if stuff.is_good() { break stuff } }; // ... once the stuff is good, break out of the loop and we'll assign that stuff to x

Understood, these things are expressions, but must they nit evaluate to a boolean in order for if/switch/match to branch correctly?

My informal understanding of a predicate may need refining.

I think the thing you're missing is that in some languages we're allowed to use these constructs as expressions. Not that they take expressions which is common in modern non-toy languages, but that they themselves are expressions.

It might be you've just always used languages where you can do this, or that you assumed you can but actually you never tried in a language you use.

In C for example we can't write this:

  int x = if (1 == 2) { 3 } else { 4 };  // Not valid C syntax

C provides a special operator, the only one in the language which has three operands and thus it's often just called "the ternary operator" although other ternary operators can technically exist, to achieve the same thing:

  int x = (1 == 2) ? 3 : 4;  // Valid C syntax using ternary operator

But, how about this?

  int x = switch 3 { case 1: 4; case 2: 8; default: 0; };   // Can't do this either

There's no way to express that in C or C++ today. The closest is the "Immediately Invoked Lambda" from C++ which is illustrated nearby in this thread. Barry Revzin apparently proposed a way to do this in C++ 26 because C++ wants to have pattern matching and pattern matching kinda sucks without this feature.

Exactly. Here is an overview of switch expressions for c#, for many cases you can replace if/else if/else with those. The pattern matching is very powerful too:

https://learn.microsoft.com/en-us/dotnet/csharp/language-ref...

"If/else expressions", as opposed to if/else merely doing control flow. Really surprising that they took so long to become mainstream, yet still more than a decade after I started using them I still occasionally miss the more "expression-like" look of the ternary in languages that don't offer both.

Apparently C++ 26 might get statement expressions in some form, it sounds like Barry Revzin sketched something at Issaquah (the recent WG21 meeting) which can do this.

you can do this in c++ as well with a lambda

  const value = [&]() {   
    if (a) {
      return true;
    }
    return switch (b) {
      case 1: return false;
      default: return undefinied;
    }
  }();

Sure, in a lot of languages this is possible, but usually much slower, even slower than named functions. Can the c++ compiler in-line the lambda and remove the function call completely?

Still the syntax is a bit confusing. In ML languages (f# and also rust are somehow descendants of ML) you can just use if .. else if .. else expressions. They work like ternaries! But easier readable. Nested ternaries are useful, but get really hard to read very fast. (x = condition1 ? A : condition2 ? B : C;)

> Can the c++ compiler in-line the lambda and remove the function call completely?

LLVM and GCC can at least.

And indeed almost exactly like that in JavaScript - just substitute ()=> for the [&]()

You can’t directly call a function definition in js, so it will look like this:

  const value = (()=>{
    …
  })()

An immediately invoked lambda pattern, but, check out how ugly that was compared to:

  let value = if a { true } else { match b { 1 => false, _ => undefined } };

And notice that because now we're a function our relationship to the world changed, how does your immediately invoked lambda do this?

  let value = if a { true } else { match b { 1 => false, 2 => return foo, _ => undefined }};

With expression syntax returning from the function we're in is fine† but in your lambda you can't do that, you need control flow logic outside the lambda to handle it.

† You might wonder about the inferred type of this expression since it might return. The type of the return expression in Rust is ! aka Never, which is an empty type. In the same way that N + 0 == N in integer arithmetic, any type plus an empty type is the original type again in type arithmetic, so the type of the expression isn't altered.

Ada calls these "if expression" and the switch-variant "case expressions". It's the same syntax as the control flow ones, just wrapped with parens.

    value := (if a then True
              else (
                 case b is
                    when 1 => False,
                    when others => Undefined
              ));

early return solves so many readability problems. i structure my code in favor of it.

Reply on news.ycombinator.com