12

I've got this error message, that I'm not a fan of.

Bad line breaking before '?'.

I feel like

var s = (a === b)
        ? 'one'
        : 'two';

looks better. Crockford says:

Semicolon insertion can mask copy/paste errors. If you always break lines after operators, then JSLint can do a better job of finding those errors.

Can someone give me an example or two, of the kind of copy/paste errors he's referring to?

Update:

var s = (a === b)
        ? 'one'
        : 'two';

looks better than

var s;
if(a === b) {
    s = 'one';
} else {
    s = 'two';
}
rythos42
  • 1,217
  • 2
  • 11
  • 27
  • 1
    If you're going to use a multi-line ternary `if`, why don't you just use a normal `if` statement? – Blender Nov 14 '12 at 22:28
  • One line expression looks better `a === b ? 'one' : 'two';`, and without extra parentheses – zerkms Nov 14 '12 at 22:28
  • 1
    @Blender: ternary operator isn't `ternary if` – zerkms Nov 14 '12 at 22:28
  • @zerkms: What's the proper name for it? – Blender Nov 14 '12 at 22:30
  • @Blender: "ternary operator" http://en.wikipedia.org/wiki/%3F: Just because there is only one - we can name it so with no additional details. And we can't do the same for binary operators (like `/` or `+`) or unary (like `++`) – zerkms Nov 14 '12 at 22:31
  • I write ternary operations like that too, looks better, more readable. It's fine, don't worry about it, just set `laxbreak=true` and solved! – elclanrs Nov 14 '12 at 22:34
  • @elclanrs As Julien Royers answers below, I'd rather not turn on laxbreak because of the return problem, which is a great reason to have the lint turned on. – rythos42 Nov 14 '12 at 22:38
  • Mmm, I've never had any problems as long as you remember the `return` case and _not_ break the line there `laxbreak` should be fine to use. – elclanrs Nov 14 '12 at 22:39
  • 2
    The "obvious" copy/paste error in the example you show would be to copy the first line `var s = (a === b)`, which of course is valid code on its own but clearly doesn't do the same thing as the three lines together. One would hope that people would look at surrounding code before copying one line, but you never know. Having said that, I don't stress about that operator in my own code. I put a short ternary expression on one line, a longer one over two lines with the line break after the middle operand and the `:` lined up under the `?`, or a really long one on three lines like yours. – nnnnnn Nov 14 '12 at 22:47
  • @nnnnnn Your example is pretty good, that shows a decent thought of why it's bad. One of my co-workers argues that this is no different than if you accidentally copied 1 line instead of 2 in any other situation. – rythos42 Nov 14 '12 at 22:55
  • The point that I think Mr Crockford is trying to make is that if you deliberately split a multi-line expression up in a way that the individual lines are not valid code on their own, then if you accidentally copy just one line of the expression it will likely cause a syntax error when you paste it somewhere else. Which is _good_ because syntax errors are reported by the browser and/or JSLint/JSHint, and so easier to find than the more subtle bugs created if you copy/paste a line that is valid on its own. – nnnnnn Nov 14 '12 at 23:34
  • @nnnnnn Can you re-post your comments as an answer? I feel that they are the best explanation to the question. Norguard, below, was close, but it wasn't until you wrote your comments that I understood what he was talking about. – rythos42 Nov 22 '12 at 21:16

4 Answers4

10

(As requested, my comments re-posted as an answer:)

The "obvious" copy/paste error in the example you show would be to copy the first line:

var s = (a === b)

...which of course is valid code on its own but clearly doesn't do the same thing as the three lines together. One would hope that people would look at surrounding code before copying one line, but you never know.

The point that I think Mr Crockford is trying to make is that if you deliberately split a multi-line expression up in a way that the individual lines are not valid code on their own, then if you accidentally copy just one line of the expression it will likely cause a syntax error when you paste it somewhere else. Which is good because syntax errors are reported by the browser and/or JSLint/JSHint, and so easier to find than the more subtle bugs created if you copy/paste a line that is valid on its own. So if you "always break lines after operators" as Crockford suggest:

var s = (a === b) ? 
        'one' : 
        'two';​

...then the only line of the ternary that is valid code on its own (the third) doesn't really look complete, and so would be easier to spot as a mistake if pasted on its own because it so obviously doesn't do anything on its own - and it's less likely to be copied by itself in the first place for the same reason.

(Having said that, I don't stress about the ternary operator in my own code, and I think the above looks ugly. I put a short ternary expression on one line, a longer one over two lines with the line break after the middle operand and the : lined up under the ?, or a really long one on three lines like yours.)

nnnnnn
  • 147,572
  • 30
  • 200
  • 241
  • Avoid copy-paste errors by putting the whole ternary into parentheses, not the condition part: var s = (a === b ? 'one' : 'two' ); – sompylasar Dec 11 '14 at 17:13
2

The most (in)famous example is as follows:

function one() {
  return
  {
    val: 1
  };
}

alert(one()); // undefined

vs

function one() {
  return {
    val: 1
  };
}

alert(one()); // [objet Object]
Julien Royer
  • 1,419
  • 1
  • 14
  • 27
  • That's the one that came up when my co-workers and I were talking about it! But it's not using the ternary operator, so no dice. :) I could see wanting the line-breaking error for this case for sure. – rythos42 Nov 14 '12 at 22:37
  • 1
    I don't think that you can come up with an ASI problem involving the ternary operator; Crockford's rule applies to all operators alike. – Julien Royer Nov 14 '12 at 22:49
2

The type of copy-paste errors he's referring to are the ones where you hand your code off to someone else, or yourself in 6 months, and that other person haphazardly copies your code, ending on the closing paren of the condition, assuming that the assignment is meant to be the value of the evaluated right-hand side.

This seems implausible, and in a sense, you would hope that it is...
But I know that auto-insertion has borked code for my company multiple times, now, and they still haven't forced adoption of explicit semicolons, still treat JS as if new lines were significant and still make cut/paste errors, through neglect plus lack of tools/version-management/build-systems.

Norguard
  • 26,167
  • 5
  • 41
  • 49
1

Say you paste a function expression in immediately before,

var a = 1, b = 1; // a === b, expect 'one'
(function(){
    console.log('called');
})
(a === b)
? 'one'
: 'two'
// called
// "two"
Paul S.
  • 64,864
  • 9
  • 122
  • 138
  • still in this case the break before "?" wouldn't matter, correct? You'd have to put a semicolon after the function expression. – mcknz Nov 14 '12 at 22:50
  • But at the same time, that everything is on different lines means anyone reading it doesn't know that behaviour isn't as intended. – Paul S. Nov 14 '12 at 22:55
  • Yeah, this definitely a confusing piece of code that would get refactored promptly at my office. :P +1 for inventiveness! – rythos42 Nov 14 '12 at 22:56
  • But your example doesn't just paste a function expression in, it also removes the `var s =` part. I think somebody pasting arbitrary function expressions like that is kind of unlikely given that it doesn't actually do anything on its own, it would need to be part of an assignment or other larger expression to be useful. Your example does create a valid larger expression, but still... – nnnnnn Nov 14 '12 at 22:57
  • 1
    Well yes, but if you're going to worry about more or less random copy and paste you've got bigger problems. (Though I suppose from time to time I have worked with people who seem to do that sort of thing.) – nnnnnn Nov 14 '12 at 23:00