remember index to reuse in else statement; avoid recalling indexOf

[elidoran]

Avoid running the same indexOf(']=') operation twice by storing the result and reusing in the else statement.

[elidoran]

Travic CI failure is due to new linting seeing unnecessary escapes. I fixed those in a new PR #184.

Also, linting required a more verbose form.

// instead of this:
var pos = (pos = part.indexOf(']=')) === -1 ? part.indexOf('=') : pos + 1;

// had to change it to this:
var pos = part.indexOf(']=') + 1;
if (pos === 0) {
    pos = part.indexOf('=');
}

[ljharb]

This isn't the same condition - the original check here was that pos was -1.

[elidoran]

Yes, I changed it intentionally. It could have been:

var pos = part.indexOf(']=');
if (pos === -1) {
    pos = part.indexOf('=');
} else {
    pos++;
}

I removed that else block by having it do the +1 in the first operation and altering the comparison value by +1, making -1 into 0.

[ljharb]

Right, but indexOf when not -1 won't necessarily be 0.

[elidoran]

When it's above zero, then it found ']=' so that's the index to use, plus 1, so the position references the equal sign, not the closing bracket, so we don't do the if statement.

What it does is:

1. looks for ']=' first, if it finds it, it adds 1 to that index (so it points at the equal sign) and uses that as the pos value.
2. if it does not find ']=', then adding one makes it a zero, and the if statement checks for zero, when it is zero, it then does the second indexOf operation to search for the first equal sign available and uses that index as pos, even if that indexOf returns a -1.

So, it will either end up with a pos value pointing at the equal sign after a closing brace, or at the first equals sign, or it's -1 (meaning neither were found).

Would you like me to change it to if (pos < 1) instead? I prefer range tests over exact comparison so I'm surprised I didn't do that to begin with.

[elidoran]

I'm not sure if i'm being clear for the spot you're looking at. The test for zero is a test for -1. It just happens to be zero because I did +1 to the result of the indexOf operation because, if it finds it, we want the +1 value so it points at the equal sign, not the bracket.

So, the if (pos === 0) is a test meaning the indexOf operation did not find ']='. If it found it, it would have returned a value of 0 or greater, which, when +1 is done, would be 1 or greater. If ']=' was the first thing in the string then the index value would be 1 to point at the equal sign.

[ljharb]

@elidoran aha, i see what you mean now. Thanks for clarifying.

I think I'd prefer the if/else for clarity, given my confusion here :-)

[ljharb]

@elidoran are you still interested in completing this PR? If so, please either check the "allow edits" checkbox in the righthand column, or make the change to if/elses and rebase :-) Thanks!

[ljharb]

(I've just noticed today that the "allow edits" checkbox is now checked, thanks)