I wrote the following test for TreeMap#split and got some unexpected behavior: https://gist.github.com/996438.
It appears the "lesser" and "greater" value sets are being reduced to singleton sets. I looked at the implementation
public P3<Set<V>, Option<V>, Set<V>> split(final K k) {
final F<Set<P2<K, Option<V>>>, Set<V>> getSome = Option.<V>fromSome().o(P2.<K, Option<V>>__2())
.mapSet(tree.ord().comap(P.<K, Option<V>>p2().f(k).<V>o(Option.<V>some_())));
return tree.split(p(k, Option.<V>none())).map1(getSome).map3(getSome)
.map2(Option.<V>join().o(P2.<K, Option<V>>__2().mapOption()));
}and I'm pretty sure the problem is in extrapolating an Ord for values (which may not be comparable at all) from the Ord provided for the keys using a comap. What the current implementation does is map each value v to (k, some(v)), where k is the split key. Unfortunately, the original Ord only considers the first part of the tuple, which is now the constant k for all values. So this comapping effectively makes all values in the set equal, and thus no value inserts after the first one.
Tony on #functionaljava mentioned that TreeMap might have taken some strong implementation cues from Haskell's Data.Map. Here's the signatures for the splits of that structure:
split :: Ord k => k -> Map k a -> (Map k a, Map k a)
splitLookup :: Ord k => k -> Map k a -> (Map k a, Maybe a, Map k a)
I'm happier with this implementation, because we're not returning back sets of values, but rather submaps, so we don't have to deal with having an Ord for values at all.
If you guys are in agreement of how to resolve this, I can put together a pull request with the fix and necessary tests.
I wrote the following test for TreeMap#split and got some unexpected behavior: https://gist.github.com/996438.
It appears the "lesser" and "greater" value sets are being reduced to singleton sets. I looked at the implementation
and I'm pretty sure the problem is in extrapolating an Ord for values (which may not be comparable at all) from the Ord provided for the keys using a comap. What the current implementation does is map each value v to (k, some(v)), where k is the split key. Unfortunately, the original Ord only considers the first part of the tuple, which is now the constant k for all values. So this comapping effectively makes all values in the set equal, and thus no value inserts after the first one.
Tony on #functionaljava mentioned that TreeMap might have taken some strong implementation cues from Haskell's Data.Map. Here's the signatures for the splits of that structure:
I'm happier with this implementation, because we're not returning back sets of values, but rather submaps, so we don't have to deal with having an Ord for values at all.
If you guys are in agreement of how to resolve this, I can put together a pull request with the fix and necessary tests.