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

Refactor all those cycles with .hasOwnProperty #34

Open
ghost opened this issue Apr 2, 2014 · 8 comments
Open

Refactor all those cycles with .hasOwnProperty #34

ghost opened this issue Apr 2, 2014 · 8 comments

Comments

@ghost
Copy link

ghost commented Apr 2, 2014

It is the slowest way to get to enumerate own properties (http://jsperf.com/object-keysv-vs-object-hasownproperty/3).

If you argue with ECMAScript3 bacward compatibility, you aren't anyway (you use Object.defineProperty to define getters and setters in certain parts of the code), not to mention, it is hard to find non-ES5 browser nowadays...

@davidgaleano
Copy link
Contributor

That is a good idea, I will give it a try.

@davidgaleano
Copy link
Contributor

Interestingly, not calling hasOwnProperty at all is significantly faster in those cases where we control the prototype chain:

http://jsperf.com/object-keysv-vs-object-hasownproperty/5

I also changed the test to use less properties which is a more common situation.

@ghost
Copy link
Author

ghost commented Apr 3, 2014

It's brittle. And for-in is looked as deprecated, in ES6 the new for-of is promoted. The brittleness comes with the fact that if you change prototype chain for some reason, mysterious bug would appear if for-in is still there.

@ghost
Copy link
Author

ghost commented Apr 3, 2014

But the speedup is massive.

@davidgaleano
Copy link
Contributor

As I said, in those cases where we totally control the prototype chain, not calling hasOwnProperty is a massive win, for any other situation I agree that using Object.keys seems like a better option.
For the record, the Turbulenz Engine already does not call hasOwnProperty in a couple of cases where performance actually matters and we are confident that the prototype chain is safe.

Sometimes we actually want the key or both the key and the value (for..in or Object.keys), whilst sometimes we only want the value (for..of), but for..of does not seem to be available everywhere yet.

@ghost
Copy link
Author

ghost commented Apr 3, 2014

FYI, for..of gives you keys, values or both depending on which iterator you call it. But yes, it's not out there yet, ES6 is not yet finished.

David Galeano wrote:

As I said, in those cases where we totally control the prototype
chain, not calling hasOwnProperty is a massive win, for any other
situation I agree that using Object.keys seems like a better option.
For the record, the Turbulenz Engine already does not call
hasOwnProperty in a couple of cases where performance actually matters
and we are confident that the prototype chain is /safe/.

Sometimes we actually want the key or both the key and the value
(|for..in| or |Object.keys|), whilst sometimes we only want the value
(|for..of|), but |for..of| does not seem to be available everywhere yet.


Reply to this email directly or view it on GitHub
#34 (comment).

@davidgaleano
Copy link
Contributor

AFAIK, for a regular object (which is what I thought we were talking about) for..of would give the property values:

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for...of

@ghost
Copy link
Author

ghost commented Apr 3, 2014

yes, yes, just, you can use k of obj.keys(), v of obj.values() or [k,v] of obj.itemsorhowisthispairiteratorcalled().

David Galeano wrote:

AFAIK, for a regular object (which is what I thought we were talking
about) |for..of| would give the property values:

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for...of


Reply to this email directly or view it on GitHub
#34 (comment).

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

1 participant