_.max incompatibility with Underscore.js about lodash HOT 15 CLOSED

lodash will only handle arrays and does not handle objects in its implementation of max.

That is by design.

Lo-Dash is optimized for the common case. I moved _.max and _.min to the Arrays category (and others as well) because all documentation and unit tests test array usage.

cc\ @kitcambridge @mathiasbynens opinions?

from lodash.

This breaks the "drop-in replacement for Underscore.js" tagline, which I feel has been a major part of lodash.

As with issue #16, I ran into this issue in my code by dropping in lodash as a replacement for Underscore.js and later getting bug reports.

from lodash.

You are using Underscore in an undocumented and untested way. I will not support this at this time but am open to revisiting it if more have this issue.

Until then you can use _.max(_.values(obj));

from lodash.

The iterator is entirely optional, per the Underscore documentation.

http://documentcloud.github.com/underscore/#max

"Returns the maximum value in list. If iterator is passed, it will be used on each value to generate the criterion by which the value is ranked."

You are correct that this is untested, as per https://github.com/documentcloud/underscore/blob/master/test/collections.js#L203, but this seems like an oversight since this scenario is explicitly supported in the max implementation, https://github.com/documentcloud/underscore/blob/master/underscore.js#L225.

I defer to to your choice of not supporting this at this time.

from lodash.

The iterator is entirely optional, per the Underscore documentation.

http://documentcloud.github.com/underscore/#max

"Returns the maximum value in list. If iterator is passed, it will be used on each value to generate the criterion by which the value is ranked."

Yap, Lo-Dash follows that too, the iterator (aka callback) is optional
https://github.com/bestiejs/lodash/blob/master/lodash.js#L1317-1367

You are correct that this is untested, as per https://github.com/documentcloud/underscore/blob/master/test/collections.js#L203, but this seems like an oversight since this scenario is explicitly supported in the max implementation, https://github.com/documentcloud/underscore/blob/master/underscore.js#L225.

It's a side effect of Underscore's design limitations. If it wants native Array#forEach it has to use _.each which also happens to be crammed with object iteration sugar. For many methods it doesn't make sense to support objects and they just happen to slip by because of the overuse/dependency on _.each.

from lodash.

Another example of Underscore's mixup/confusion is its _.shuffle method which is stated to shuffle an array, but also internally uses _.each and so indirectly supports objects.

from lodash.

Computing the maximum and minimum values for an object is a bit of an edge case, but it makes sense—unlike, say, shuffling an object's properties.

from lodash.

values for an object is a bit of an edge case

Yap, in this case I don't think support was intended and is just a side effect of Underscore's overloaded API which we are moving away from with the introduction of _.forIn and _.forOwn.

from lodash.

Mixup/confusion, side effect of design limitations or intentional choice - they do at least group all of these in their "Collections (Array or Objects)" section.

from lodash.

Mixup/confusion, side effect of design limitations or intentional choice - they do at least group all of these in their "Collections (Array or Objects)" section.

Underscore's design problems aren't limited to its API or implementation and also extend to its organization of methods. I disagree with several of the method category assignments and have adjusted them to better reflect real-world/common usage.

See also Underscore's invoke (under "Collections") vs. Lo-Dash's invoke (under "Arrays").

from lodash.

@thorlarholm Also, I just wanted to let you know that I am totally thankful for your feedback and previous feedback/bug reports. Because Lo-Dash doesn't have many of the restrictions of Underscore it gives me the opportunity to rethink some of the design (method categories, api, edge cases, etc.). I'm trying to strike a balance between Underscore compatibility and optimizations/clarifications for common usage.

Please let me know if _.max(_.values(obj)); will work for you and of any other issues you have in the future.

from lodash.

@jdalton I'm happy to give feedback, no hurt feelings here for not getting this changed; expect more feedback.

The alternate implementation is just as fine for me, and a bit more explicit at that.

from lodash.

As of Lo-Dash v0.8.2 _.max, _.min, and _.shuffle work with objects.

from lodash.

Nice, thank you for the diligent work.

from lodash.

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

from lodash.