Flotype / now

NowJS makes it easy to build real-time web apps using JavaScript
http://www.nowjs.com
MIT License
1.91k stars 175 forks source link

hasOwnProperty quirk #53

Closed dvv closed 13 years ago

dvv commented 13 years ago

Hi!

https://github.com/Flotype/now/blob/master/lib/nowUtil.js#L86:

Sometimes causes the falsy negatives -- i.e. I pass a hash which surely is created from scratch (by literal notation) and the function returns false. When I change it to be Object.hasOwnProperty.call(currVar, prop) the result is positive.

Notice, that in CoffeeScript (which compiles to a "least common denominator" JS code, which runs on every JS engine), they use namely the said signature -- Object.hasOwnProperty.call(obj, prop) to determine whether a property is own.

I'd recommend to introduce a helper function __isOwnProp(obj, propName){return Object.hasOwnProperty.call(obj, propName)} and rewrite all invocations of obj.hasOwnProperty(prop).

Sorry for being didactic, and TIA, --Vladimir

sridatta commented 13 years ago

Thanks @dvv. We'll change it wherever we make that call. Do you know why a direct call to hasOwnProperty is not consistent?

dvv commented 13 years ago

Have no idea so far. I guess because obj.hasOwnProperty itself can be overridden. update: they use __hasProp = Object.prototype.hasOwnProperty (notice prototype)

update2: exactly because obj.hasOwnProperty itself can be tainted/overridden

update3: this might be the reason why this change fixes the issue: there also exist objects which have no hasOwnProperty in their prototype -- the most prominent is null ;)

ericz commented 13 years ago

Javascript is a silly language. I suppose this is something that should be changed. Iteration through objects is a mess in this language.

dvv commented 13 years ago

Sure. Two options: rely upon underscore, which is all about convenient iterations over "lists", normalizing hashes and arrays; or CS which has clean syntax for that ;) The third option is to very carefully code inline patterns, or reinvent own set of helpers -- this usually leads to nowhere in the long run.

erichocean commented 13 years ago

@dvv Why not fork and write you own CoffeeScript version? Sounds like it's so easy and convenient that you'd be done in a few hours.

dvv commented 13 years ago

Of course not. I'm just trying to help biting problems I see. Plus, it will be useless unless it's accepted into the master. The whole point I'm asking whether CS source is feasible to have is to explore the field of your opinions. I'm not trying to force using CS in any way.

ericz commented 13 years ago

Thanks for both of your contributions guys. Dvv has been very polite in helping us out. More awareness of best practices can't hurt.

ericz commented 13 years ago

@dvv I have changed all obj.hasOwnProperty to Object.hasOwnProperty.call

Is there a benefit of using Object.prototype.hasOwnProperty.call vs Object.hasOwnProperty.call ??

dvv commented 13 years ago

Thanks!

I guess the latter costs one more hop along the prototype chain. They just teach us augmenting/mangling Object.prototype is bad practice, so they assume Object.prototype.hasOwnProperty is pristine in either case.