Hacker News Reader

Comment by AYBABTME | Hacker News Reader

I've once come across:

   if (!Boolean.FALSE.equals(aBoolean)) { 
      // ...
   }

I was pretty baffled.

View on HN

I've seen a surprising amount of code that does:

    if (aBoolean == true) {
       ....
    }

At university I worked with small unmanned aircraft. We had a crash due to a piece of code with this form:

  int landing_flag;
  ...
  if( landing_flag ) {
    do_landing();
  }

It was C code that pre-dated a boolean type. A single corrupt data packet in a wireless link made landing_flag == 2345923 (some arbitrary large value) and thus the landing routine was triggered mid-flight.

We changed every instance of if( flag ) to if( flag == specific_flag_value ) to ensure that particular bug didn't rear it's ugly head again. I keep doing that now.

So, you changed a somewhat reproducible bug into one that has about a four billion times lower chance of occurring?

Good luck to the poor chap who will have to figure out what happened when that bug hits.

Also, you introduced a new error condition: a corrupt packet that should set a value of 1, but arrives as a value of 2 will not initiate the landing routine.

The right thing to do, IMO, is to prevent corrupt data packets from doing such stuff. Checksum the packets or, better yet, checksum and encrypt them. That prevents the enemy from taking over your plane.

Finally, I do not see how 'no proper bool' is relevant here. If the packet contained a single bit indicating the value of the flag, it still could get corrupted.

> So, you changed a somewhat reproducible bug into one that has about a four billion times lower chance of occurring?

Yes. You seem to imply that's a bad thing?

> Also, you introduced a new error condition: a corrupt packet that should set a value of 1, but arrives as a value of 2 will not initiate the landing routine.

A corrupt packet should not do anything, so that's good, not an error. We do not want the landing routine to be accidentally triggered in flight. Missing a valid packet is much better than triggering on an invalid packet. (It's a UDP protocol, so the entire system is designed to handle missed packets. Ground station re-sends commands until positive acknowledgement is received from the aircraft)

> The right thing to do, IMO, is to prevent corrupt data packets from doing such stuff. Checksum the packets or, better yet, checksum and encrypt them. That prevents the enemy from taking over your plane.

Exactly right. We were already using a checksum in the datalink, and the corrupted packet that caused the crash passed the checksum as valid! During the post analysis of the crash, I discovered that it was using an 8-bit XOR checksum implemented years earlier. 8-bit XOR is ok for detecting single bit errors, but is not good at detecting burst errors -- it does not detect ~12% of highly corrupted packets. I also updated the system to use a significantly more robust checksum after that incident.

It was a bad thing, the way you described it. Now that I know you also fixed the root cause of the problem, I can see it as an additional line of defense.

I think I wouldn't add it, though. Time is better spent on tooling that checks the variable doesn't get an incorrect value.

Like you said, an additional line of defense. For a web application there's no need. For a flight critical application where a failure means you just lost a few $100k worth of hardware, then I'll take every measure possible.

But that's not quite the same - older versions of C didn't have a real Boolean type so you could well run into those kinds of problems.

In a language that does have a proper Boolean type I still think checking equality with literal true/false values is a bit silly.

Typically that isn't done from ignorance, just some (imho misguided) idea about readability.

Probably the same kind of place that bans use of the ternary conditional operator because it's "too complicated" <sigh>

Eh, I can't say that I have any love for the ternary if operator. It is an ugly construct made necessary by the other flow control constructs being statements. Scala lets you just use if/else like that, the result is much clearer at the cost of 4 or so characters.

Guilty as charged. Although it's practically the same, I prefer the TRUE part. Same with the !aBoolean and FALSE.

I saw this in some coding standard for PHP.

Where it actually makes sense (assuming === and not == is used for comparison).

eh, that's not so bad really.

If I were writing something, and I really wanted to make it clear that I was testing a boolean to be true, I might write that. Then I can be absolutely certain the person reading it in 5 years won't misread it.

Actually, the more I think about it, the more I'd be inclined to do that in the false case ( ie if(aBoolean == false)... rather than if(!aBoolean).. ) because I worry that it's too easy to skip over the '!' - and I personally read that as "not aBoolean" rather than "aBoolean is false"...

I'll bet it complies to the same thing anyway, so it's just about readability at this point.

Why stop there? To really get the point across, you may want to do:

if (((((a == true) == true) == true) == true) == true)

That way, you can be even more certain!

People have different preferences when it comes to coding styles. There are some that are subjective (balance between clarity vs. simplicity), and there are idiotic styles such as !Boolean.FALSE.equals(aBoolean), or if (((((a == true) == true) == true) == true) == true).

The point is that using (a == true) instead of a is idiotic. Now, there are good semantic reasons to use (a == true) in some languages, but we're not talking about that, we're talking purely about style. It's no different from doing (a && true) or (i + 0) or (f * 1.0) when a, i and f would suffice. Not only is it longer, it forces you to stop and think, was there a good reason why this was being done? Can a hold truthy values that the programmer's trying to exclude? Otherwise, it's a sign of a programmer that doesn't know what he's doing - I've spent a decent amount of time teaching and grading and you see this a lot in first-year school assignments, rarely in high-quality production code. In almost all cases, it's a holdover from someone going in their head "if a is true" (because "if a" doesn't read well in human languages) and writing that out, not some kind of conscious effort to make the code more readable.

> In almost all cases, it's a holdover from someone going in their head "if a is true" (because "if a" doesn't read well in human languages) and writing that out,

It's interesting you admit that people are doing it "the long way" because that's how it naturally flows out of their head.

Doesn't it make sense that it would naturally flow into their head the same way?

What's the goal here - write very tight, concise code that fits some arbitrary standard of "correct"?

Or to write code that flows out of and into people's heads easily?

I was describing first-year students who are reading code as though it's written in a foreign language, not anyone with some degree of competence. For obvious reasons, professionals should not be internally verbalizing as they write or read code, any more than a fluent French speaker should be internally translating from English as they speak French.

Potentially, I can construct an independent statement to adequately convey my innervation to your statement of factual basis, that while being grammatically correct, purposefully adds needless complexity to the original statement of intent I am adequately attempting to convey.

Should I?

What are you talking about?

If it's somehow clearer to say if (x == true) instead of if (x), why isn't it also clearer to say if ((x == true) == true) instead of merely if (x == true)? And so on?

Huh. You sound just like me 5-10 years ago, everything black and white.

There is a grey zone in the middle, move in there.

There's grey zone and there's being obtuse. We're not talking about fancy tricks here - the clearest way to retrieve the truth value of a boolean variable is to refer to it directly, not comparing it for equality against a constant.

That might be the clearest way for you to write it today, though I'm talking about it being clear for someone else to read many years from now.

As so many posts on HN have said lately; writing code is easy, it's the reading that's difficult.

How is if (x == true) easier to read than if (x)? Deliberate obtuseness makes things harder to read.

Read my original comment. It would very much depend on context, and, as I said, I'd be more inclined to do it in the false case.

It's funny, you still seem to think there is a "right and wrong" here, and you can't see that coding style is just like writing a poem - each individual will do things a little differently.

What context does it depend on? Where is it reasonable to say (x == true) instead of x?

Edit: your original post said "If I were writing something, and I really wanted to make it clear that I was testing a boolean to be true, I might write that." What does this even mean? Simply writing x isn't really really clear, so you write (x == true) to make it extra clear? if (x) is testing boolean to be true and it's crystal clear. What else can it mean?

I personally think it depends on the complexity of the current function, and the complexity of the variable x. i.e. if x has been read and written to a bunch of times in the function already, it might add clarity to be very clear that you mean "if x is false".

The clearest way I can think of to write that, that's hopefully less prone to misinterpretation is if(x == false) rather than if(!x)

Like I said, these are just my personal opinion and an expression of how I code (and likely things that I find difficult or often misread when reading the code of others)

I've seen undergrads trying to write Ada:

  case x is
    when true => y;
    when others => z;
  end case;

You know, so the code doesn't stop compiling when someone comes along and adds values to the Boolean type.

Well, FileNotFound is a popular addition from what I hear.

Came across this one yesterday, slightly paraphrased:

  boolean updateMsg(boolean pPassed) {
    if (pPassed) {
      incrMsgCounter();
      logger.info("Message sent");
    } else {
      logger.info("Failed to send");
      return false;
    }
    return true;
  }

Yes, the calling code actually checked the return value. The code is full of stuff like this. Somehow I've got a morbid fascination and can't stop marvelling at how grotesque it is. It even overpowers the urge to read HN.

What's so bad about this one?

Other than its returning a boolean being pointless, I mean?

As 1stamour suggested the whole function was pointless. Not a big deal by itself but if you have layers upon layers of this stuff, code quickly becomes a buggy, unmaintainable mess. There are some awesome static code analysis tools like PMD and PHPMD for measuring code quality. It's not just "all programmers think everyone else's code is crap".

The code doesn't return just any boolean. No, it returns the boolean it was passed. And the calling code checks the result, what, against the boolean it passed in, I wonder? I'd worry about unintended side-effects depending on the language...

Reply on news.ycombinator.com