whats wrong with this ternary operator?

Question

I have an object menuNames which should maintain a list of menu items. If menuNames already has the slug, increment the value, if it doesnt contain the slug, set the value equal to 1. I'm doing this to track unique names. I want to end up with something like:

menuNames: {
    home: 1,
    products: 10,
    contact: 1
}

this doesnt work (this would be contained in a loop going through each slug):

menuNames[slug] = (menuNames.hasOwnProperty(slug) ? menuNames[slug]++ : 1);
//this sets every value to 1

but this does work (this would be contained in a loop going through each slug):

if(menuNames.hasOwnProperty(slug)) {
    menuNames[slug]++;
} else {
    menuNames[slug] = 1;
}

You could also just write: `menuNames[slug] = (menuNames[slug] || 0) + 1`. — Ferdinand Beyer, Feb 08 '13 at 16:53

score 8 · Accepted Answer · answered Feb 08 '13 at 16:49

menuNames[slug]++ increments the value, but also returns the original value.

You are doing menuNames[slug] =, so the value is set back to the original value after being incremented.

To fix it, just simply do:

menuNames[slug] = (menuNames.hasOwnProperty(slug) ? menuNames[slug]+1 : 1);

Or:

(menuNames.hasOwnProperty(slug) ? menuNames[slug]++ : menuNames[slug] = 1);

insertusernamehere · Answer 2 · 2013-02-08T16:57:42.800

3

I guess it could work like this:

menuNames[slug] = (menuNames.hasOwnProperty(slug) ? ++menuNames[slug] : 1);

edited Feb 08 '13 at 16:57

answered Feb 08 '13 at 16:49

insertusernamehere

23,204
9
87
126

3

This would work, but it seems inefficient. `++menuNames[slug]` increments the value and returns the new value, which is then assigned to the variable. Why do 2 assignments? – gen_Eric Feb 08 '13 at 16:52

score 2 · Answer 3 · answered Feb 08 '13 at 16:52

2

As the other answers say the problem is in the post increment.

Another way to write it is:

menuNames[slug] += (some_bool ? 1 : 0);

++ is very sensitive to bugs. Try to write it as a += statement.

if menuNames[slug] can be undefined, write it as:

menuNames[slug] = 0;
if (some_bool) {
    menuNames[slug] += 1;
}

This is (in my opinion) the clearest way to write an initialization/counter loop.

If you like one-liners you'll cringe, but if you like bug free code you'll be happy to see this.

answered Feb 08 '13 at 16:52

Halcyon

57,230
10
89
128

1

This wouldn't work if `menuNames[slug]` was `undefined`, as `undefined+0` is `NaN`. – gen_Eric Feb 08 '13 at 16:54
Added a case if `menuNames[slug]` can be `undefined` – Halcyon Feb 08 '13 at 16:56

whats wrong with this ternary operator?

3 Answers3