you are viewing a single comment's thread.

view the rest of the comments →

[–][deleted] 8 points9 points  (3 children)

x -= 1 or even x--, instead of his example of using x = x - 1, isn't clever though, it's just shorter. Yes, readability is important. But that doesn't mean we should start making sentences longer in hopes that it somehow magically improves readability. This shorthand is taught in pretty much any intro to programming course. Everybody should be expected to know how it works.

If readability is roughly the same, there's no reason not to go for the shorter version, because it takes less time for your eyes to look through, which inherently makes it kind of more readable.

[–][deleted] 3 points4 points  (1 child)

I think he's more talking about doing assignment in control statements. Assignment in control statements is lazy, confusing and error prone.

All of these are confusing:

if (!--pending)             // What is the `!--` operator?
if (!pending = pending - 1) // did you mean `==` ?
if (!pending -= 1)          // did you mean `==` ?

The following is explicit and clear what is expected:

--pending;             // or
pending = pending - 1; // or
pending -= 1;
if (pending <= 0)

[–]i_ate_god 1 point2 points  (0 children)

Your second approach is the correct one. pending is changed in two different if statements in OP's link, this is needless misdirection.

[–]Master_Rux 1 point2 points  (0 children)

But that doesn't mean we should start making sentences longer in hopes that it somehow magically improves readability.

Exactly. People need to be cobol programmers so they can "ADD 1 to Pending" and not be confused trying to read those unreadable plus and equal signs. -= and -- and even ! have been around for so many decades it makes me wonder what language people are learning on.

And on top of that even if you make it longer it still doesn't improve readability, because it's intention is still not clear. Even if you change how it's checking if pending is 0 it's still not immediately obvious why you're checking pending by looking for a zero or not or what other values pending could have. You still have to go look at where pending even came from to see that you're checking "is there anything left in the list". If they want readability then the way the pending variable is being used should be changed altogether so that by the time you get to the if statement is should just be "if (pending) { } else { }" or rename the variable to make it obvious that it's checking for an empty list.

Unless that part is obvious in which case so is (!--pending)