To actually use a Set Rust type internally for Edn::Set I had to do several changes, they are:
I tried using HashSet, however since both HashMap and HashSet don't implement Hash, they can't be used as keys on HashMaps and/or values on HashSets. Since this ain't' viable, I went with the BTreeMap/BTreeSet solution, which is the same as the mentat/edn crate;
Also since HashSet<Edn::Double> exists now, Edn::Double suddenly had to implement Hash. To make this happen I've used the ordered_float crate, just like the mentat/edn did;
Also I had to do some serialization and macro internal changes, because of the new types that I am using.
Considerations on API Design
There is one thing I would like a second opinion. Before to use the variant Edn::Double, you could just pass a f64 to it, like Edn::Double(1.2).
Now since we will be using the OrderedFloat, I've created a public alias Double which just stands for OrderedFloat<f64>.
The problem with this is that the user has to either do Edn::Double(Double(1.2)) or it can do Edn::Double(1.2.into()) (more pleasant in my opinion). Do you think this is a problem/issue? If you are okay with the user having to do this conversion or use the new alias type then I think we can proceed normally.
By the way this is probably a breaking change, since Edn::Double(1.2) won't work anymore, and will have to be substituted by Edn::Double(1.2.into()). But still, is a very simple fix, and it doesn't affect macros at all, just the creation of Edn manually via enum š
Also, any suggestion is always welcome š , the project is yours after all hahaha
To actually use a
SetRust type internally forEdn::SetI had to do several changes, they are:HashSet, however since bothHashMapandHashSetdon't implementHash, they can't be used as keys onHashMaps and/or values onHashSets. Since this ain't' viable, I went with theBTreeMap/BTreeSetsolution, which is the same as the mentat/edn crate;HashSet<Edn::Double>exists now,Edn::Doublesuddenly had to implementHash. To make this happen I've used theordered_floatcrate, just like the mentat/edn did;serializationandmacrointernal changes, because of the new types that I am using.Considerations on API Design
There is one thing I would like a second opinion. Before to use the variant
Edn::Double, you could just pass af64to it, likeEdn::Double(1.2). Now since we will be using theOrderedFloat, I've created a public aliasDoublewhich just stands forOrderedFloat<f64>.The problem with this is that the user has to either do
Edn::Double(Double(1.2))or it can doEdn::Double(1.2.into())(more pleasant in my opinion). Do you think this is a problem/issue? If you are okay with the user having to do this conversion or use the new alias type then I think we can proceed normally.By the way this is probably a breaking change, since
Edn::Double(1.2)won't work anymore, and will have to be substituted byEdn::Double(1.2.into()). But still, is a very simple fix, and it doesn't affect macros at all, just the creation ofEdnmanually via enum šAlso, any suggestion is always welcome š , the project is yours after all hahaha
Fixes https://github.com/naomijub/edn-rs/issues/3