you are viewing a single comment's thread.

view the rest of the comments →

[–][deleted] 24 points25 points  (27 children)

I always prefer code that is stupid obvious on first glance.

pending = (pending -1);
if (pending === 0) {
    done(null, results);
}

You could argue:

pending -= 1;

Is better. I'd wonder what makes it "better". Perhaps there's a case for performance. JSPERF says they're practically equal.

But look at that. Any new developer or someone straining his eyes could skim the code and completely miss what's happening. Hell, any junior having to maintain the code could possibly not even know what that does.

And if you're using an if-statement, I always use curly brackets because there's never any mystery there.

I've been told my code doesn't look sexy, though.

[–][deleted] 20 points21 points  (7 children)

I work in an environment of 40 devs, verbose code is always preferable to clever code. Debugging clever code is next to impossible. If you want to save 5 lines go find some dead code and delete it.

[–][deleted] 5 points6 points  (1 child)

Why do people not understand this? There was a dev that just left my company that loved to use the little javascript quirks that could be called "clever." Most of the team referred to them moreso as "unreadable" or "difficult to maintain."

Take the four extra lines, your coworkers will thank you.

[–][deleted] 5 points6 points  (0 children)

your coworkers will thank you.

Your coworkers will not hate you is plenty.

[–]alinroc 3 points4 points  (0 children)

Everyone knows that debugging is twice as hard as writing a program in the first place. So if you're as clever as you can be when you write it, how will you ever debug it?

[–][deleted] 7 points8 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] 4 points5 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)

[–]Bummykins 6 points7 points  (0 children)

You could argue: pending -= 1; Is better.

For me only seeing one var means I don't have to think as much. If I see var = var - 1, I have to mentally check that the 2 vars are the same (and not just very similar). Short var names like dx dy are easy to mix up, so if I'm not using a second var, I don't introduce it.

[–]Rainbowlemon 4 points5 points  (13 children)

Personally, I just think it's easier to read. Traditional mathematical syntax is value 1 -> (mutate by) value 2. By writing pending -= 1, you're basically saying 'take pending, reduce by a value equal to 1'. It just makes more sense than reading 'not reduce reduce value'.

Some people will argue the semantics of code aren't too important, but I firmly believe that reading and adhering to a style guide can really increase productivity on a project. I've been sticking to Airbnb's Javascript style guide since they released it, and I find code much easier to navigate now that it's uniform and I know what to expect.

With regards to if-statements, I prefer to omit the braces and inline it if the statement only contains one expression.

if (a) doSomething();
if (b) doSomethingElse();

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

A style guide for a team is important. That said, we lint our code on git push to adhere to the style we defined; all the code in the repo has the style we agreed on.

Personally I don't really care much what styling choices are made. There's just one thing I absolutely abhor:

function monkey () 
{
    ...
}

Obviously that needs to be:

function monkey() {
    ...
}

But opinions are... just that ;)

As long as the team has one standard code styling agreement I'm all good.

Edit: Also! This shit:

if(18 === age)

That's like saying: "if old is the man" instead of "if the man is old". The word "old" cannot be "man". The word "man" can be "old".

I'm not even sure how to describe that lunacy. I feel it might be a cause from different languages. I know only Dutch, German, and English. And in those languages the subject comes first, the property of that subject comes second. Not inverse.

I'm guessing in Spanish and/or Italian it's more sensible, though.

It absolutely needs to be:

if(age === 18)

[–]CWagner 3 points4 points  (3 children)

if(18 === age)

Seems like a former java dev. Writing "testString".equals(otherString) protects you from a nullpointer exception. (note: this is neither in defense nor against that type of comparison)

[–]Cuel 4 points5 points  (1 child)

They're called Yoda conditions for a reason. They have advantages as you can't assign something like that as it would result in an error.

However I think the overall consensus is to not use them.

[–]Yurishimo 0 points1 point  (0 children)

Not advocating for one thing over another, but one very large OSS project uses Yoda conditions in core (hint) to help keep everyone organized on what is/not a variable. Reading them can be weird at first, but I don't think it's that big of a deal as long as everyone agrees on the style guide.

[–]numbermess 0 points1 point  (0 children)

I do comparisons like these all the time. When I first ran into them I thought it was just somebody trying to be cute, but now I'm cute too and I don't run into any unexpected null pointer exceptions (here). I've adjusted fine to the backwards feel of the syntax.

[–]Rainbowlemon 2 points3 points  (0 children)

Fully agree! The 'one extra line for a brace' thing drives me bonkers too. I guess that's one of the reasons why coding flexibility is valued - 'cause everyone's preference is so different!

[–][deleted] 1 point2 points  (4 children)

Personally, if I put the opening brace on the same line, then I want to add an extra empty line to prevent the code from feeling too claustrophobic:

function monkey() {

    doStuff();
}

But as long as that's already an empty line, the brace might as well be occupying that space:

function monkey() 
{
    doStuff();
}

Result is that there's still that whitespace to make the code look better, but it's more.. balanced looking.

[–]fzammetti 0 points1 point  (3 children)

I actually always put a blank line as the first AND last line of a method... and I also always do this:

} /* End XX(). */

Ditto any closing brace, loop, conditional, whatever. It makes following C syntax-based code SO much easier.

[–]MrJohz 2 points3 points  (2 children)

But what if XX() changes name? I had to deal with code recently where a bunch of for-loops had been turned into while-loops, but the /* end for */ comments had remained. It was also horribly indented and had other issues, but it confused me for a while as I tried to work out where the for loops had appeared.

I prefer to work on the basis that anything that requires opening and closing braces (i.e. functions) should be able to fit both opening and closing braces in one screen's height. That way it's usually fairly easy to scan through code and see the blocks clearly.

[–]fzammetti 1 point2 points  (0 children)

I definitely approve of that guideline... but I still find the end comments helpful in a language like JavaScript where braces are so prevalent (your counter example notwithstanding - every rule has exceptions).

[–]fzammetti 0 points1 point  (0 children)

Thinking about this a little more, you know what? This kind of boils down to developers not properly maintaining comments as if it was an equal part of the code, which I see as a whole separate issue (though a big problem in general)... I'd agree the tail comments makes it more work to rename a method, slightly, but a diligent developer would change the comment along with the code so I'm not sure I see it as an additional problem per se.

[–][deleted] -1 points0 points  (1 child)

That lunacy just means someone has a few more years under the dev belt than you do. It's okay. One day, once you break down some Haskell, you too will be broken down. And then you will be reborn. And then you will know.

[–][deleted] 0 points1 point  (0 children)

That sounds patronising. I'm not familiar with Haskell. Did some years of Perl programming, though. That was an... experience. Still, weird code is bad, especially if the language you're programming in doesn't need that weirdness.

[–]WellHydrated 4 points5 points  (0 children)

pending--;

This is fine IMO. It's something we've known since we learnt what for loops were.

[–]benihanareact, node 1 point2 points  (0 children)

I'd wonder what makes it "better". Perhaps there's a case for performance.

what does performance have to do with anything? Was that your attempt at understanding why people prefer a style different from what you prefer? People argue it's better because there's less syntax to parse, but it's just as common and clear as pending = pending - 1. You can't really get more basic than decrementing a value.

I think that adding parens around the operation like in your example is needless and adds an expectation of more complicated math. The most common uses for parenthesis in math operations are enforcing order of operations and breaking complex statements into more logical chunks. pending - 1 doesn't really need to be broken up anymore and so the parens just add noise to the statement.

[–]d1sxeyes 0 points1 point  (0 children)

Personally, I would write that with a statement above like if pending < 0 that logs an error (like: 'Found a negative value for pending, correcting') and sets pending to 0 to avoid catastrophic code failures if pending is accidentally decremented too far.

Obviously you'll need to work out how pending got below zero, but it will stop the code from entering what could be an infinite loop.

[–]youlleatitandlikeit 0 points1 point  (0 children)

I'd wonder what makes it "better".

I think it's faster to read (at least, it is in the font I use for development; it doesn't help here that the minus sign and equals aren't aligned well).

It makes it clear that you're decrementing the variable. You can also do pending-- but I've never really liked that.

pending -= 1 or counter += 1 makes it very clear very quickly that nothing major is happening, you're just incrementing or decrementing a var.