Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

This is all very good and very, ahem, true. But (and it's a big butt);

if (need_free == true)

Is such a horrible code smell to me. You have a perfectly good boolean. Why compare it to a second boolean to get a third boolean?

if (need_free)

or

if (!need_free)

for the opposite case is so much better.

I will admit that in my world this leaves

if (need_free == some_other_bool)

as something I don't have a particularly comfortable way of doing safely.



Better example:

  #define FLAG_63 (1ULL << 63)
  long long flags = FLAG_63;
In this case,

  if (flags & FLAG_63) pass();
will pass, but

  typedef int BOOL;
  BOOL set = flags & FLAG_63;
  if (set) pass();
won't pass, due to truncation.

Question: Would you argue that a datatype that holds the smallest (1-bit) datum should be as wide as the largest integer type just to handle such cases?

If so, that would be highly inefficient for storage purposes. Note that Win32 has 32-bit BOOL type, but internally NT uses 8-bit BOOLEAN type to store bools in structures.


  > if (need_free == true)
  > Is such a horrible code smell to me. You have a perfectly good boolean. Why compare it to a second boolean to get a third boolean?
  > if (need_free)
You are probably interested if the `need_free` flag is set to true, and not if `need_free`. It is true that `if (need_free)` has the same behaviour, but it is some steps farther from what you are interested in.


This feels to me like you're introducing the same unnecessary extra layer into your text as in the original code. I mean, why not

"You are probably interested in whether it's true that the 'need_free' flag is set to true"

leading to

> if ((need_free == true) == true)

? Answer: because that extra layer of indirection adds nothing, and just gives you a bit of extra cognitive load and an extra opportunity to make mistakes. I think the same is true about going from "need_free" to "need_free is set to true".

(This becomes less clear if you have variable names like 'need_free_flag'. I say: so don't do that then! It's almost always appropriate to give boolean values and functions that return boolean values names that reflect what it means when the value is true.)


Couldn't we do it like (!need_free == !some_other_bool)?


You can go the php way and do if (!!need_free)


Clever, I like it.


For:

  if (need_free == some_other_bool)
you could use:

  if (!need_free ^ some_other_bool)
if you're using _Bool.




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: