search

addyosmani / memoize.js

A faster JavaScript memoizer
http://addyosmani.com
143 stars 19 forks source link

Resolve issues for Underscore.js pull #8

Closed addyosmani closed 10 years ago

addyosmani commented 12 years ago

I've taken the work we've done so far and put it into a pull request for inclusion in Underscore.js as a viable replacement to their _.memoize() method: https://github.com/documentcloud/underscore/pull/330 (all tests pass).

Performance tests comparing the most recent version of memoize.js to underscore's implem are here from today: http://jsperf.com/comparison-of-memoization-implementations/12

If you take a look at the comments from Jeremy, his main concerns at the moment are that we've dropped the second argument in the memoize signature they use (the hasher function). He's also concerned about the dependency on the JSON object being present, and our reliance on it for stringification.

We do have an open issue attempting to solve this with an alternative, but as you might know, a reliable stringification method needs to cover lots of use cases and can't (as far as I know) be distilled down to something which they might consider including. With this in mind, I've got some questions:

  1. Do you guys think we should pursue trying to get this into Underscore?.
  2. What are your thoughts on a replacement to the JSON stringification we're doing at the moment? (I would look at the comments from Jeremy on the pull).
  3. If we do factor in an optional hasher function, do you think we should apply this to just the pull request or this repo too?. I mentioned in the request, but I haven't really seen too many people need the second argument.

//cc @JamieMason @DmitryBaranovskiy

Cheers! Addy

addyosmani commented 10 years ago

As it's been a few years since this issue, let's close it. If someone wants to tackle it in the future we should open a new issue up.