szymonrw / ancient-oak

Immutable data trees in JavaScript.
MIT License
223 stars 11 forks source link

Feature: eq function #6

Open jasonkuhrt opened 10 years ago

jasonkuhrt commented 10 years ago

I think we need an .eq(...) function that will accept any

and figure out if the values are the same.

jasonkuhrt commented 10 years ago

Here is an example deep_equal implementation https://github.com/chaijs/chai/blob/b7fdf4919e7fb166e4afdb7d549da6dd9be34e13/chai.js#L491-L520

jasonkuhrt commented 10 years ago

@brainshave Could data.json() === JSON.stringify({...}) work? Seems like we'd get bad performance or errors if Dates or Functions were in the data structure? Oh and JSON would throw errors on any cyclic properties which seems bad too.

szymonrw commented 10 years ago

I think you mean data.json(). Interesting idea but it won't work because keys in objects can be in a different order.

About dates: not supported yet. About functions: a function inside immutable tree is treated as a data node. Ancient Oak is pretty much about storing data, not functions (module interfaces and such) ;-) My usual thinking is if I can fit in JSON I can fit it in Ancient Oak (apart from dates, for now). With cycles (and thus a tree becoming a graph) it's too easy to suddenly have different versions of what is supposed to be the same thing if you start modifying the data. (the solution for that problem would something like Clojure's atoms)

As for .eq it'll be better if it is a function taking two elements in the Ancient Oak module, not a method on data objects. Then we can easily provide a method if there's a need for that.

Is this something you want to work on or just a suggestion? It's fine either way :-)

jasonkuhrt commented 10 years ago

I think you mean data.json().

Yep.

Interesting idea but it won't work because keys in objects can be in a different order.

You're totally right from a spec perspective but that hasn't stopped some people from relying on key order: https://github.com/natefaubion/matches.js. But that's just trivia, I agree a robust solution cannot rely on key order.

As for .eq it'll be better if it is a function taking two elements in the Ancient Oak module, not a method on data objects. Then we can easily provide a method if there's a need for that.

Sounds good, lets go with that

Is this something you want to work on or just a suggestion? It's fine either way :-)

Both : ). I would love to take a shot at it some time in the near future. I'm working on a prelude library that needs this feature (because I intend to build atop ancient oak).

jasonkuhrt commented 10 years ago

This looks pretty good https://github.com/chaijs/deep-eql.

What would be the issues with e.g.:

var deep_equal = require('deep-eql')

oak.eq = eq

function eq(a, b){
  deep_equal(oak(a).dump(), oak(b).dump())
}

I'm assuming there would be a performance issue wherein the data tree would be traversed twice, first by ancient-oak, then by deep_equal. An implementation specific to ancient-oak should be able to do this in one pass.

szymonrw commented 10 years ago

Not only traversed but also cloned unnecessarily. Yeah, we shouldn't recreate whole trees only to compare them.

Funny thing: implementing ao-specific version would require probably only changing accessing fields with object[field] to object(field). It could be much simpler also than deep-eql, given constraints in ao (only plain arrays, dates, objects and primitives).