4

If I recall correctly from Crockford's "Javascript: The Good Parts", he is not in favor of using the ++ or -- operators, but I also tend to recall he doesn't provide an especially strong argument against them.

Below is a use of these operators that I've found helpful in keeping my code as concise as possible particularly when dealing with functions/methods that return -1 when 0 is the first possible valid return value (along with positive integers). I'll be interested in other atypical uses of ++ and/or -- that make a strong argument in favor of using these operators when they make sense.

I do not consider this a duplicate of Why avoid increment ("++") and decrement ("--") operators in JavaScript? but rather it's corollary: when to not avoid them, but rather use them to your advantage. Of course I could be mistaken and there could be some reason I'm not thinking of as to why the following is fraught with danger despite seeming to me to be elegant -- if I'm missing something sub-optimal about the following, I'd like to know that too 

var substrIdx = str.indexOf(substr);
if (++substrIdx) {
  doSomething(--substrIdx);
}
Community
  • 1
  • 1
Dexygen
  • 12,287
  • 13
  • 80
  • 147
  • 2
    Having seen Crock speak a couple times I'm pretty sure I can accurately imagine his response to that example :-) – Pointy Nov 09 '11 at 00:11
  • 2
    Your example doesn't quite make sense: `substr()` returns a string, not an index; it would work for `indexOf()`. But anyway, while I _like_ the increment and decrement operators in general I don't like that sort of use because it is much harder to read. Anybody else seeing your code will have to stop and think about how it works (especially if you don't include a comment), whereas `if (substrIdx != -1)` is immediately obvious. – nnnnnn Nov 09 '11 at 00:14
  • You're correct about substr I meant indexOf I will update the code to reflect that – Dexygen Nov 09 '11 at 00:16
  • 1
    Crockford states "*We don't need a dedicated operator for adding one to things*". I agree. – Šime Vidas Nov 09 '11 at 00:19
  • 1
    P.S. What if there is code after the if block that uses `substrIdx` - at the moment the value only gets changed back if the if evaluates as true. More generally, while we don't _need_ a dedicated operator it does exist and everybody knows what it means, so why not use it (but not in a way that obfuscates the code)? – nnnnnn Nov 09 '11 at 00:21
  • Ok, my code sucks, LOL. So what about other uses of ++ or -- that make good sense? – Dexygen Nov 09 '11 at 00:24
  • What @nnnnnn said about blocks of code using substrIdx afterwards... very good point. This goes to maintainability for sure. – Scott A Nov 09 '11 at 00:24
  • 1
    Crockford is correct about not needing these. In fact, if it weren't for the fact that the PDP-8 had pre- and post-increment instructions, C never would have, and none of the C-derived languages (*i.e.*, everything in current use except LISP dialects and COBOL) would either. – Ross Patterson Nov 09 '11 at 00:27
  • If you're going to ask about when it's OK to use incrememt and decrement, you should post an example that would be considered OK by most people. What you have is really cryptic and would not pass a code review by me. Right now, it just sounds like you really really want to use it, but don't have a good reason. – Ruan Mendes Nov 09 '11 at 00:30
  • 1
    Not "needing" a particular operator is not a good argument against it. We don't _need_ `for` either, since we have `while`. We don't _need_ `+=` since we can say `x=x+1`. So what? Write your code so that others can read it easily and you should be fine no matter what operators you choose. – nnnnnn Nov 09 '11 at 01:12

4 Answers4

5

Concise is not the same thing as readable or maintainable, and elegance is in the eye of the maintainer. The code you posted seems unnecessarily opaque to me.

Over the past couple of decades I have developed an intense aversion to code that isn't immediately obvious, even if the obvious method requires a good bit more code. Implicit and magic code is almost always less maintainable if more than one person is in the mix.

A specific comment: I don't see why you consider your code to be more concise, elegant, or readable than:

var substrIdx = str.indexOf(substr);
if (substrIdx >= 0) {
  doSomething(substrIdx);
}

It's also a tiny bit less efficient because you're performing two operations and a comparison instead of just a comparison. Additionally you're mistreating an integer as a boolean which is almost always a bad idea from a maintenance standpoint.

Finally as mentioned below you're hindering Javascript minimizers, and from a standpoint of conciseness at the end the only thing the user cares about is how quickly the page loads...

Scott A
  • 7,745
  • 3
  • 33
  • 46
4

If I'm missing something sub-optimal about the following, I'd like to know that too

var substrIdx = str.indexOf(substr);
if (++substrIdx) {
  doSomething(--substrIdx);
}

It's sub-optimal. It is confusing to humans, requires shipping more code over the browser, and requires the interpreter to do more work at runtime.

Someone trying to figure out what it is, has to reason about the state of substrIdx at different points in the code -- "it obeys the contract of indexOf here, but there it's one greater than the contract of indexOf, and after the if it doesn't have a clear relationship to substr.".

It's also longer than

var substrIdx = str.indexOf(substr);
if (substrIdx>=0) {
  doSomething(substrIdx);
}

is harder for minifiers to minify because substrIdx is no longer a single assignment var. A good minifier will turn the latter into something like

var substrIdx = str.indexOf(substr);
substrIdx<0||doSomething(substrIdx);

but fewer will do the job with the unnecessary assignment. They compare pretty well under closure compiler:

var a=str.indexOf(substr);++a&&doSomething(--a)

vs

var a=str.indexOf(substr);a>=0&&doSomething(a)

because closure compiler misses the opportunity to turn a>=0&& into a<0|| but the increment and decrement is still longer.

Community
  • 1
  • 1
Mike Samuel
  • 118,113
  • 30
  • 216
  • 245
  • 1
    Your example shows what the OP's code is really doing in a much clearer way. Plus, in the example by the OP, `substrIdx` is going to be one larger than it was after the call to `indexOf`, which is probably not intended – Ruan Mendes Nov 09 '11 at 00:27
2

Well, that's surely "atypical". It's also "clever". In the "boy, I hope I never have to support code written like that" vein of "clever".

Code should be readable. We spend much more time reading code than writing it. This isn't.

Ross Patterson
  • 9,527
  • 33
  • 48
  • Not to kick off a language war, but this is why I don't like Perl. Too many opportunities for write-only code, which I think is what this example displays. – Scott A Nov 09 '11 at 00:28
  • Well at least my cleverness is within the span of a couple of lines. But I whole-heartedly agree in retrospect: my code sucks! The only thing that might make it salvageable would be to use it as interview question: re-write this code! – Dexygen Nov 09 '11 at 00:32
  • 1
    @GeorgeJempty: If you like to trick people in interviews, go ahead and use it, I refuse to work for a place that tries to ask tricky questions. I prefer to ask meaningful questions and go through their thought process – Ruan Mendes Nov 09 '11 at 00:37
  • It wouldn't be about tricking people. You need to know your operators: even if they're considered sub-optimal, you will encounter them in the wild. You also need to know how to re-factor code, and this seems like a dirt simple example: a half dozen people here have succeeded. I think you could probably rule out somebody who didn't know these operators and/or couldn't make a decent attempt at improving the code, or didn't at least have the fundamental knowledge of String methods such as indexOf – Dexygen Nov 09 '11 at 00:42
0

The only case I consider it acceptable is when the result of the increment is not being stored, therefore it doesn't matter if it's pre or post-increment and won't be a source of bugs.

 var j=0;
 // These three examples are OK in my mind
 for (var i=0; i < 20; i++) {
    doSomething()
 }

 while ( i < 100) {
   if (someTest()) {
      j++;
   }      
   i++;
 }
Ruan Mendes
  • 90,375
  • 31
  • 153
  • 217
  • Please leave a comment when you vote down! I know this is not atypical, but it's the only use I consider safe – Ruan Mendes Nov 09 '11 at 01:04
  • Not the downvoter, but maybe they voted down because you didn't really explain why other use cases are bad (besides a blanket "source of bugs"). – Brendan Long Nov 09 '11 at 01:30