Use hasOwnProperty throughout the code

[DHainzl]

Hi there,

I just upgraded less from 1.4.1 to 1.6.0 and it did not work immediately. In my environment, the Array.prototype was extended with a "unique" function, which resulted in a lot of error messages, for example in those two lines (in the compiled less-1.6.0.js):

:1956 name[k].charAt is not a function -> name["unique"] did not exists
:4424 "mixin" is not defined -> source was loop in :4417 and then access to candidates["unique"]

Adding

if (Object.prototype.hasOwnProperty.call(name, k)) { /* ... */ }

to all for ... in loops made it work for me.

Yours, David

[seven-phases-max]

Hmm, interesting, I suspect it's mostly my fault.

if (Object.prototype.hasOwnProperty.call(name, k)) { /* ... */ }

I guess then it's better to change all such loops to a regular for (i; i < n; i++).

---

Selfnote: L1956 is already fixed in #1764

[lukeapage]

yes lets switch on the jshint option too for it

[lukeapage]

@seven-phases-max are you working on this?

[seven-phases-max]

No, I'm not. Honestly I don't know how to find all such loops (I did not know that jshint has an options or this). So if you are going to take it, cool! But let me know if you prefer me to do that.

[seven-phases-max]

😞 Tried it with "forin": true. It just warns about every for ... in not counting if it's used with an array or with an object (though now I understand it's actually impossible to detect it with jshint selectively for obvious reasons).
For example: https://github.com/less/less.js/blob/master/lib/less/functions.js#L669 - these for ... in seems to be correct (or do I miss something?). Well, of course it's not a big problem to convert all for ... in loops to for (i++) (or to for ... in + isOwnProperty where applicable) but it feels a bit stupid to not use shorter versions.

[matthew-dean]

For...in is unfortunately very fragile with JavaScript objects because the members are so malleable, and often you don't REALLY mean to iterate through all members (including ones extended by other code, such as in @DHainzl), just a specific set. It shouldn't feel stupid, and it's usually a bad idea to use them with arrays. Or to use them at all, in my experience, lol. Same thing with hasOwnProperty, which is also fragile but less so. With objects or arrays, specificity is important, or if you use for...in to at least check if the object member returned is the type you expected.

[matthew-dean]

Also, IIRC, there's no performance improvement to using for...in over for (i++) so, with all the hassle involved, I just avoid the former these days.

[seven-phases-max]

I don't care of perfomance I just hate cluttered ifs and macaroni {{{}}} :). (Just in case I complain about objects of course, arrays are fine since for (i++) exists. Now I start to think that I will tend to unintentionally convert all objects to arrays just to avoid if (variable.hasOwnProperty(k)) { /* ... */ } :)

[Synchro]

for..in is sometimes much slower because it's for enumeration, not iteration which I think is where the confusion arises as they are the same in many other languages (e.g. PHP). If you don't care about order you can go backwards which allows you to work with only a single local var:

for (var i = thing.length - 1; i >= 0; i -= 1) {...

and if you do want to go forwards you need the extra var, and the model is:

for (var i = 0, j = thing.length; i < j; i += 1) {

[lukeapage]

we had around 10 places using for-in and only 1 that was on an array. the 2nd place mentioned in the description was already fixed.

I replaced the array one with a for(i = 0; i < a.length; i++) - unless you iterate over millions of items, caching the length adds more in complexity and extra characters than you save. In our case most of our arrays are under 10 items.

the for-in for object doesn't have a browser compatible replacement afaik since Object.keys isn't in older browsers. Could rely on a polyfill, but thought I may as well leave as-is and add a hasOwnProperty to protect against hostile environments where a plonker has addded something to the object prototype.