Why doesn't short hand decleration work in a for of loop JS

pops64 0
Viewed 46
    const charCount = {}
    for (const char of modString1){
        // if (charCount[char] === undefined)  charCount[char] = 1
        // else charCount[char]++
        charCount[char] = charCount[char]++ || 1
    }

So I have this for loop that I am iterating over each character of a string to generate the number of times it repeats. The commented out code works. My understanding charCount[char] = charCount[char]++ || 1 is the same but the code doesn't work it always returns 1 for every character. I have also tried the ternary operators as well with the same result. Anyone have answer to this quirk?

javascript
2 Answers

If x starts at 0, then x = x++ || 1 will always set x to 1, never allowing it to advance further. This is because the result of x++ is x before incrementing, which is 0 on the first invocation, so the || 1 kicks in and the entire right-hand side evaluates to 1.

The second (and every subsequent) time through, x++ || 1 is always 1; even though x++ then sets x to 2, you're setting it back to 1 when the assignment operator is evaluated (because again, x++ evaluates to whatever x was before being incremented).

You need x = (x + 1) || 1.

The fundamental problem here is that you're effectively doing x = x++. This is a weird and incorrect thing to do. You're taking a simple assignment, and turning it into two assignments: x++ is itself an assignment, it mutates x, and you're mixing this with a second assignment, which mutates x a second time, undoing the first assignment. One of those mutations should not exist. You should typically see x++ as a statement, or x = x + 1 as a statement, but not both together, that is almost always incorrect.

Don't use ++ here. It's the wrong place for it, it's not the tool for your job.

user2290440

As user229044 answered, assignments of form x = x++ are redundant, because we are effectively discarding the increment. This is because a postfix increment evaluates to its old value.

Your assignment can therefore be rewritten as: x = x || 1 or in words, if x is falsy set it to 1.
This is not the intended behaviour.

If we refactor your commented out if-statement, we can more easily see a pattern behind it:

if (x === undefined) x = 0;
x++;

As you can see, we want to treat the nullish undefined as zero, and always increment (upon finding a certain character).

Your final code may look like:

x = (x || 0) + 1;

Sidenote: Your if-statement only fell back when x was undefined, but your refactor fell back when x was falsy, which has broader meaning. This is technically irrelevant since x is always either a number or undefined. But using the nullish coalescing operator ?? instead of the OR operator || conveys our original intention more clearly: x = (x ?? 0) + 1

Note that converting undefined to a number results in NaN. Because NaN is falsy, the OR operator || will replace it with 1 as intended regardless (cite: user229044's comment). Therefore, using a prefix increment works too:

x = ++x || 1;
Oskar Grosser0
Related