Two bugs found (cycle.js): duplicate obj refs handling, primitive Object-form handling

[getify]

Two bugs found in the decycle() method in "cycle.js":

1. 'decycle()' replaces all duplicate object references, instead of (as implied by the name) only those which actually create a circular (aka, 'cycle') reference.
2. decycle()' treats all the Object forms of the primitives (Number, String, Date, etc) improperly.

[getify]

This brings decycle() more into line with the behavior its name implies (removing cycles, only). It also brings it more into conformance with how JSON.stringify() itself works, which is to serialize all duplicate refs found (just fails on circular refs).

[douglascrockford]

The behavior is as intended.

[getify]

Can you explain why you want a = {b: new String('foobar')} to behave differently compared to a = {b: 'foobar'}? And can you explain why you would want that different behavior for decycle() as opposed to just how JSON.stringify() natively works?

var a = { b: new String('foobar') };
var c = { d: "foobar" };

JSON.stringify(a); // {"b":"foobar"}
JSON.stringify(c); // {"d":"foobar"}

JSON.stringify(JSON.decycle(a)); // {"b":{"0":"f","1":"o","2":"o","3":"b","4":"a","5":"r"}}
JSON.stringify(JSON.decycle(c)); // {"d":"foobar"}

Just struggling to understand why THAT is "intended behavior"?

[smt]

This is clearly a bug. Please fix.

[douglascrockford]

The bug Kyle identifies as bug 1 is not a bug. I am not going to accept a change that is half wrong. When he has a correct request, I will consider it.

[getify]

New PR: https://github.com/douglascrockford/JSON-js/pull/41

[getify]

I apologize for being "half wrong". I assumed that the common definition that most programmers are familiar with for the word "cycle" (aka, a "circular reference") is the one you were using when you named the function decycle(). I assumed thus that the purpose of decycle() was to remove "circular references" (and moreover, to do ONLY that task).

But, from https://github.com/douglascrockford/JSON-js/blob/master/cycle.js#L26

Make a deep copy of an object or array, assuring that there is at most one instance of each object or array in the resulting structure. The duplicate references (which might be forming cycles) are replaced...

It would appear that you are using an alternate definition for the word "cycle", one which actually means "any duplicate object reference, whether it's a cycle/circular reference or not." Fine.

Thanks for re-educating us all on what the word "cycle" actually means in this programming context.

[douglascrockford]

Kyle ‏@getify apparently i'm crazy enough to open myself up to blunt ridicule from crockford:

Kyle ‏@getify oh, no! now i'm really stirring the pot, daring to disagree with crockford, with.. ya know.. facts:

Kyle ‏@getify ha! classic crockford:

Kyle ‏@getify the crockford shell script would more or less be a great turing test candidate. we should hook it up to a site like crockfordspeaks . com

Kyle ‏@getify better: "AskCrockford . com", and of course, someone needs to get @AskCrockford to hook up to it as well.

Kyle ‏@getify if only i worked for a company that gave me "20%" time i could totally build the digital automated crockford suite.

Kyle ‏@getify haha, it just keeps getting better from crockford. legitimate and obvious bugs "as intended":

Kyle ‏@getify i knew crockford was crazy bad, but i am dumbfounded by this:

Kyle ‏@getify I'm grateful that crockford is re-educating us on what the word "cycle" means WRT object references

[getify]

Yes, I have been tweeting about my frustrations with this (and other) threads with you. Mostly because I wanted other people to chime in.

I came to this project with good faith in the attempt to improve the code, and I think you have disregarded and disrespected that good faith by how you've treated my efforts.

[douglascrockford]

Your behavior has been childish.

[WebReflection]

Kyle, I believe consistency has been, and should be, a key in JSON protocol. If you create primitive wrappers there's no way to encode/decode these in a meaningful way exactly because "foo" is different from new String("foo") and for many reasons.

It is actually bad in my opinion that JS developers are still using the new keyword before String, Number, and Boolean, and specially with latest 2, new Boolean(false) and new Number(0) are truthish conditions and completely unexpected.

Moreover, primitives cannot possibly have properties ... var s = "abc"; s.prop = 123; alert(s.prop) would result into undefined while var s = new String("abc"); s.prop = 123; alert(s.prop); would result into 123 and you see that here you could have cycling loops.

If you use objects then you might want to represent them as object one you deserialize these objects otherwise you loose properties and this would be as bad as loosing properties with any other object.

What decycle does in this case is, in my opinion, half correct in any case because, specially with strings, the length property is missing so that object, once parsed, cannot represent in a meaningful way the object it was during serialization ( JSON.stringify ) while a length property could make, at least the String case, less ambiguous as ArrayLike Object.

I know you guys here are aware of everything I wrote but at least other developers might understand that using new for primitive constructors is a bad practice. Sorry to interrupt or to interfere with this thread, br.