search

JWambaugh / TJSON

The tolerant JSON parser for Haxe
78 stars 25 forks source link

Removed cache from the special Date object. #21

Closed ciscoheat closed 8 years ago

ciscoheat commented 9 years ago

I think the encoded Date object is small enough not to be cached, so it can be self-contained. It happened to me when using TJSON-encoded data in a REST service that the Date was cached but the cached object didn't exist in the response. What do you think?

ciscoheat commented 9 years ago

So what do you say? :)

ciscoheat commented 8 years ago

Another bump on this :)

JWambaugh commented 8 years ago

This change is breaking unit tests. I'll merge it once it's not breaking tests. do an sh runTests.sh to run the tests (unix/mac)

ciscoheat commented 8 years ago

Yes, unit tests can be at a too low level sometimes. But if not, is it a requirement that cached Date objects are keeping their references after serialization?

ciscoheat commented 8 years ago

Sorry, deserialization (as done in the testObjectReferences test)

JWambaugh commented 8 years ago

That's not why the test is failing. It looks like the reference indexes are getting mixed up.

But as a user, I think I probably would actually want references to continue to be references and not copies.

Perhaps an option to use reference caching or not would be best.

ciscoheat commented 8 years ago

Well, my patch removes Date objects from caching altogether, so it will never match any references. But since it's on a cache level, originally from JSON which has no references, I don't see it as a problem for Date at least, which is probably a value object in most users minds anyway. :)