Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document time complexity #3

Closed
joebandenburg opened this issue Apr 12, 2015 · 4 comments
Closed

Document time complexity #3

joebandenburg opened this issue Apr 12, 2015 · 4 comments

Comments

@joebandenburg
Copy link

From my reading of the code, you're using arrays to store the keys and values and searching them linearly in get and set. I think you should document in your README that the polyfill has worst algorithmic complexity than a real Map implementation. The polyfill insert and lookup operations are O(n) whereas any decent native implementation will have O(1) (or perhaps O(log n)) operations. This difference may matter to your users.

@medikoo
Copy link
Owner

medikoo commented Apr 12, 2015

@joebandenburg this is how you would naturally implement it (without overthinking optimisation in first place). It's also how ECMAScript specifies the lookup, (it's O(n)). So it's not a deviation from spec we're doing here, or other lie that needs to be exposed.

Still, of course lookup can be internally more optimal, and there's already opened issue for that. See #2

@medikoo medikoo closed this as completed Apr 12, 2015
@brianmhunt
Copy link

@medikoo : @joebandenburg is correct. See my question on point, including (for example, from the ES6 spec):

Set objects must be implemented using either hash tables or other mechanisms that, on average, provide access times that are sublinear on the number of elements in the collection

So using O(n) array lookup differs from the requirements of the spec, and so probably worth noting (IMHO).

Cheers

@medikoo
Copy link
Owner

medikoo commented Jun 29, 2015

@brianmhunt yes, but it's not possible to obey to that paragraph without altering the objects which are added as map keys, and for frozen objects it's not possible at all.

So if you look at it from right perspective, every Map polyfill should contain similar note :)

Anyway since v1 es6-map will do O(1) for most (not frozen of course) cases, I agree it's acceptable in ES5 world (it defnitely wouldn't in ES3 world, but this polyfil doesn't support ES3)

@medikoo
Copy link
Owner

medikoo commented Jan 11, 2017

I noticed many people trying to use this library expected O(1), therefore I added note as requested to the readme.
It'll be addressed with v1, but due to limited time I can devote into it, I'm not able to state any certain date when it'll happen

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants