Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

if statements – one line vs. one expression #445

Open
tomek-he-him opened this issue Jul 30, 2015 · 5 comments
Open

if statements – one line vs. one expression #445

tomek-he-him opened this issue Jul 30, 2015 · 5 comments

Comments

@tomek-he-him
Copy link

Until I started using ESLint with your configuration I saw nothing against the rules in this:

$ cat if.js
const [one, two] = [1, 2];

if (one !== two) throw new Error(
  'One does not equal two'
);

But ESLint does:

$ eslint if.js 

if.js
  3:0  error  Expected { after 'if' condition  curly

✖ 1 problem (1 error, 0 warnings)

Is this intended? In my opinion the pattern I’ve been using is explicit – and more readable than this:

if (one !== two) {
  throw new Error(
    'One does not equal two'
  );
}

The parens form a visual brace-like block much like in #438.

@ljharb
Copy link
Collaborator

ljharb commented Jul 30, 2015

Our rules seem to require either:

if (one !== two) throw new Error('One does not equal two');

or, as an alternative to your last example:

if (one !== two) {
  throw new Error('One does not equal two');
}

@tomek-he-him
Copy link
Author

Yeah, both of your examples look good with a five-word error message. I use my variant because my error messages tend to get long and descriptive – much like in the #438 example.

So it’s a question of:

if (one !== two) throw new Error(
  'One does not equal two. This error is common when you try to determine if ' +
  'the number `1` has the same value as the number `2`. Please try again, or ' +
  '– if that fails – get in touch with your administrator.'
);

vs.

if (one !== two) throw new Error('One does not equal two. This error is common when you try to determine if the number `1` has the same value as the number `2`. Please try again, or – if that fails – get in touch with your administrator.');

vs.

if (one !== two) {
  throw new Error(
    'One does not equal two. This error is common when you try to determine ' +
    'if the number `1` has the same value as the number `2`. Please try ' +
    'again, or – if that fails – get in touch with your administrator.'
  );
}

The second is obviously invalid. And I do prefer the first to the last – for the sake of readability.

But feel free to close the issue if you don’t think my point makes sense – I’ll adapt.

@tomek-he-him
Copy link
Author

Just one more thought to support my suggestion.

<a>(<b>, <c>);

is semantically the same thing as:

<a>(
  <b>,
  <c>
);

– you just use one or the other depending on the length of the <a>, <b>, <c> expressions. When maintaining code you often switch between the first and second form.

It matters to me whether a chunk of code is one expression or statement, not whether it fits in one line.

Therefore, these should also be equivalent in any case:

throw new Error('One does not equal two');
throw new Error(
  'One does not equal two'
);

– whether they stand freely, after an if statement, or anywhere else there’s place for one statement.

@goatslacker
Copy link
Collaborator

If we go with this for #438 then we should apply the same rule for if statements.

Our rule makes the distinction that if it fits on one line then it's good to omit braces, otherwise we add the braces.

@ljharb
Copy link
Collaborator

ljharb commented Aug 22, 2017

@englishextra your comment is both unhelpful and condescending; I'm going to delete it. Please be respectful in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants