7 ms·

I've once come across: if (!Boolean.FALSE.equals(aBoolean)) { // ... } I was pretty baffled.

prodigal_erik 13y ago
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.
- jsight 13y ago
  Well, FileNotFound is a popular addition from what I hear.
arethuza 13y ago
I've seen a surprising amount of code that does: if (aBoolean == true) { .... }
- jlgreco 13y ago
  Typically that isn't done from ignorance, just some (imho misguided) idea about readability.
  - arethuza 13y ago
    Probably the same kind of place that bans use of the ternary conditional operator because it's "too complicated" <sigh>
    - jlgreco 13y ago
      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.
  - sigkill 13y ago
    Guilty as charged. Although it's practically the same, I prefer the TRUE part. Same with the !aBoolean and FALSE.
  - chanux 13y ago
    I saw this in some coding standard for PHP.
    - andyroid 13y ago
      Where it actually makes sense (assuming === and not == is used for comparison).
- asynchronous13 13y ago
  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.
  - Someone 13y ago
    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.
    - asynchronous13 13y ago
      > 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.
- grecy 13y ago
  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.
  - candybar 13y ago
    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!
    - grecy 13y ago
      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?
      - candybar 13y ago
        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?
        grecy 13y ago
        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.
        13y ago
drderidder 13y ago
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.
- krapp 13y ago
  What's so bad about this one? Other than its returning a boolean being pointless, I mean?
  - lstamour 13y ago
    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...
  - drderidder 13y ago
    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".

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.

jsight 13y ago
Well, FileNotFound is a popular addition from what I hear.

arethuza 13y ago

I've seen a surprising amount of code that does: if (aBoolean == true) { .... }

jlgreco 13y ago
Typically that isn't done from ignorance, just some (imho misguided) idea about readability.
- arethuza 13y ago
  Probably the same kind of place that bans use of the ternary conditional operator because it's "too complicated" <sigh>
  - jlgreco 13y ago
    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.
- sigkill 13y ago
  Guilty as charged. Although it's practically the same, I prefer the TRUE part. Same with the !aBoolean and FALSE.
- chanux 13y ago
  I saw this in some coding standard for PHP.
  - andyroid 13y ago
    Where it actually makes sense (assuming === and not == is used for comparison).
asynchronous13 13y ago
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.
- Someone 13y ago
  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.
  - asynchronous13 13y ago
    > 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.
grecy 13y ago
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.
- candybar 13y ago
  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!
  - grecy 13y ago
    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?
    - candybar 13y ago
      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?
      - grecy 13y ago
        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.
        13y ago

drderidder 13y ago

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.

krapp 13y ago
What's so bad about this one? Other than its returning a boolean being pointless, I mean?
- lstamour 13y ago
  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...
- drderidder 13y ago
  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".