john-n-smith / mvcobject-js

An implementation of the KVO pattern, inspired by Google Maps API v3 MVCObject class
7 stars 1 forks source link

What is the test at the start of .set() supposed to do? #1

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
I added a comment to line 193 indicating I think there is mistake:

http://code.google.com/p/mvcobject-js/source/browse/trunk/MVCObject.js#193

The comment appears to indicate that one isn't allowed to set a property that 
isn't already set, but then how does one set a value the first time?

Original issue reported on code.google.com by James.Sy...@gmail.com on 16 Sep 2012 at 4:23

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
The set() method is for assigning a value to property that has already been 
defined, not for defining new properties.

Consider the example on the homepage; The property ObjectA.foo has already been 
defined on the object, prior to bindTo() or set() being called.

Original comment by j...@urbanplum.eu on 17 Sep 2012 at 9:32

GoogleCodeExporter commented 9 years ago
Thanks for the clarification about purpose of set.

My comment on line 193 was addressing a different issue.  The code says:

if (!key in this) ...

Which is equivalent to:

if ((!key) in this) ...

Which for most keys is ALWAYS going to be equivalent to:

if (false in this) ...

I think you wanted:

if (!(key in this)) ...

Original comment by James.Sy...@gmail.com on 18 Sep 2012 at 8:46

GoogleCodeExporter commented 9 years ago
Ah! Thanks - that exception would almost never have been raised.

I'll update and push now.

Thanks again,

John

Original comment by j...@urbanplum.eu on 18 Sep 2012 at 9:37

GoogleCodeExporter commented 9 years ago
I'll handle this one.

Original comment by j...@urbanplum.eu on 18 Sep 2012 at 9:49

GoogleCodeExporter commented 9 years ago
Fixed in latest commit. Downloads need updating though.

Original comment by j...@urbanplum.eu on 18 Sep 2012 at 9:51