_.max gets incorrect results in extendScript

[RaymondClr]

//In Underscore.js 1.13.2
var stooges = [{name: 'moe', age: 40}, {name: 'larry', age: 50}, {name: 'curly', age: 60}];
var result = _.max(stooges, function(stooge){ return stooge.age; });
result.age
=> Result: 40

After my troubleshooting, I found that the problem first appeared in version 1.7.0

//In Underscore.js 1.6.0
var stooges = [{name: 'moe', age: 40}, {name: 'larry', age: 50}, {name: 'curly', age: 60}];
var result = _.max(stooges, function(stooge){ return stooge.age; });
result.age
=> Result: 60

[jgonggrijp]

@RaymondClr Thanks for reporting.

I don't have the means to reproduce the problem. Your code example returns 60, as it should, with Underscore 1.13.2 in Safari. _.max is also passing the unittests in Node.js and several browsers, which include examples like yours.

I looked at the diff between 1.6.0 and 1.7.0, but I see no obvious change that might have broken _.max for ExtendScript. However, that is likely due to my limited knowledge of ExtendScript.

(The diff link opens on the wrong part of the page. You have to click on the "Files changed" tab, scroll all the way to the bottom and click "Load diff" on underscore.js. The scrolling position will then jump to _.max, which I've highlighted.)

Concluding, I believe you, but I have no means to address the problem myself. Does ExtendScript have a debugger and if so, could you step through your example code to find out where things are going wrong?

[RaymondClr]

@jgonggrijp Thank you for your reply. First of all, I am sorry that my English is not very good. The following content is the result of Google translation. I hope it can express my meaning correctly.

extendScript has a very old debugging tool (its official download page is not even accessible anymore), called Adobe ExtendScript Toolkit CC, its debugging function is very poor, so I can only troubleshoot problems by printing intermediate variables in the program, but In the end, I found the cause of the problem. The logic of extendScript (based on es3) in parsing the ternary operator seems to be different from the conventional implementation. I inserted the following print content in the _.max source code of Underscore version 1.7.0:

  _.max = function(obj, iteratee, context) {
    var result = -Infinity, lastComputed = -Infinity,
        value, computed;
    if (iteratee == null && obj != null) {
      obj = obj.length === +obj.length ? obj : _.values(obj);
      for (var i = 0, length = obj.length; i < length; i++) {
        value = obj[i];
        if (value > result) {
          result = value;
        }
      }
    } else {
      iteratee = _.iteratee(iteratee, context);
      _.each(obj, function(value, index, list) {
        computed = iteratee(value, index, list);
        $.writeln("computed & lastComputed: " + computed + ' ' + lastComputed)
        $.writeln("computed > lastComputed: " + (computed > lastComputed))
        $.writeln("computed > lastComputed || computed === -Infinity && result === -Infinity: " + (computed > lastComputed || computed === -Infinity && result === -Infinity))
        $.writeln("computed > lastComputed || (computed === -Infinity && result === -Infinity): " + (computed > lastComputed || (computed === -Infinity && result === -Infinity)))
        if (computed > lastComputed || computed === -Infinity && result === -Infinity) {
          result = value;
          lastComputed = computed;
          $.writeln("lastComputed: " + lastComputed)
        }
      });
    }
    $.writeln("result: " + result.age)
    return result;
  };

Here is the output from the ExtendScript Toolkit CC console, notice the difference in the output before and after adding the parentheses:

computed & lastComputed: 40 -Infinity
computed > lastComputed: true
computed > lastComputed || computed === -Infinity && result === -Infinity: true
computed > lastComputed || (computed === -Infinity && result === -Infinity): true
lastComputed: 40
computed & lastComputed: 50 40
computed > lastComputed: true
computed > lastComputed || computed === -Infinity && result === -Infinity: false
computed > lastComputed || (computed === -Infinity && result === -Infinity): true
computed & lastComputed: 60 40
computed > lastComputed: true
computed > lastComputed || computed === -Infinity && result === -Infinity: false
computed > lastComputed || (computed === -Infinity && result === -Infinity): true
result: 40
Result: 40

Although I don't know what the root cause is (it may be related to the details of extendScript lexical analysis), the solution to this problem is simply to add a pair of parentheses:

if (computed > lastComputed || computed === -Infinity && result === -Infinity) {
    //...
}

Change to

if (computed > lastComputed || (computed === -Infinity && result === -Infinity)) {
    //...
}

_.max can output the correct result.

[jgonggrijp]

Excellent research @RaymondClr, thanks a lot!

Now that we know that ExtendScript has the relative precedence of || and && wrong, we should add parentheses everywhere these operators occur in the same expression. If you like, you can submit a pull request with such a change and get the credit. Otherwise, I'll do it.

If you choose to prepare your own pull request, please make sure to run npm install before editing the code. This will install a commit hook that keeps the bundle in sync with the modules. Edit the modules (i.e., the files in the modules/ directory) rather than the bundle directly.

[jgonggrijp]

@RaymondClr I just found out that the minifier is already disambiguating expressions with && and || by adding parentheses. Could you confirm that the issue only happens with the unminified version of Underscore?

[dogbubu]

gsdg54sdg5s4d5gドっぱくラバニラサダウのブはとてもおいしそうなりんかマカョ♂