Are unbracketed conditionals considered hacky?

    if (records[index]) {
      cb(null, records[index])
    } else cb(new Error("User " + id + " doesn't exist"));

The else clause will run because JS will run whatever is after the keyword.

This also works for if statements with no other else/if clauses, that are on a single line.

I personally like this condensed syntax as it’s a bit more compact and clean. Do you guys think it’s hacky though? If it looks bad to recruiters I wouldn’t want to use it.

Usually when people say “hacky” they mean something else. Many developers would consider this to be sacrificing readability without really gaining anything. On professional projects, where an entire team must be able to read and edit the same code, there are often fairly strict styling requirements to make the code consistent and easy to read.

A recruiter will not be looking at your code.

2 Likes

readability and consistency is most important

  if (records[index]) cb(null, records[index]);
  else cb(new Error("User " + id + " doesn't exist"));

or

    if (records[index]) {
         cb(null, records[index])
    } else {
         cb(new Error("User " + id + " doesn't exist"));
    }

i personally find the 2nd way to be more readable. I think most companies would prefer that too. but i think companies go by their own predefined standard

if you’re not gonna use brackets with else then you shouldn’t use them at all
it should make enough sense at a glance without having to read too deep into it

1 Like

well… inconsistent, but not really hacky, from a recruiters point of view, looking at that code would make me think, he/she was doing things too quickly or even, not being aware of good code principles.

My little coding brain grew up on the 2nd too. I was in the habituation stage of dropping braces but I think i will just stick with them in all cases. JS is already complex as it is.

PS: That first set of statements you wrote will never reach the else clause (dropping the braces forces it to end with a single statement)

Install the Prettier plugin for your editor, set it for format on save, then forget about it, it’s much easier to not need to worry about bikeshedding stuff.

it all comes down to your style of coding and how you adapt to change, when working on a team project it’s better to keep a consistent style guideline for everyone to follow vs personal liking and convenience.
I like to do it this way dropping the curly braces makes it cleaner if the body of if-statement can fit on one line

if(some condition) 
   // do something if it's a one-liner
else
   // do something here just a one-liner

Well, and the body of an if statement can ALWAYS fit into one line. All you have to do is wrap the body of a multi-line if statement in a function, and then call that function. For example:

if(foo !== bar){
  // We'll swap foo and bar here, why not?
  let baz = foo;
  foo = bar;
  bar = baz;
}

That can be single-lined simply by:

function swapFooAndBar(){
  let myThrowawayVar = foo;
 
  foo = bar;
  bar = myThrowAwayVar;
}

if(foo !== bar)
  swapFooAndBar();

It’s picking nits, but if the company design manual defined that all if/else statements were to be un-bracketed, then this would be the way: each possible branch of each if/else would have to be defined as a function, then called as a single-line if body.

1 Like

I think/hope in most larger teams there will be linters and auto formatting in place and some commit hook stopping you from deviating from the adapted style guides.

As for the original question, any formatting that looks like a potential error is, in my opinion, a bad idea. I’m OK with a short one-liner if statement, but then I always put it on one line.

Also, plus one for using prettier.

1 Like

Is prettier sometimes buggy for you guys? The below route handling callbacks both have the same number of arguments but look at the formating on the 2nd one:

app.get("/logout", function(req, res) {
  req.logout();
  res.redirect("/");
});

app.get("/profile", require("connect-ensure-login").ensureLoggedIn(), function(
  req,
  res
) {
  res.render("profile", { user: req.user });
});

Pretty sure it’s not a bug but the max 80 characters per line rule.

1 Like