Skip to content
This repository has been archived by the owner on Oct 4, 2020. It is now read-only.

split #102

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

split #102

wants to merge 2 commits into from

Conversation

matthewleon
Copy link
Contributor

@matthewleon matthewleon commented May 31, 2017

addresses #26

Note that this probably has nlogn rather than logn performance, as in Haskell's collections. It can probably be improved very significantly. Would like to add some benchmarks (#101) before doing so.

@paf31
Copy link
Contributor

paf31 commented Jun 3, 2017

I think this is good, but we should probably try to implement it by walking the tree, which would be more efficient. This is great for a first pass though, thanks!

@@ -451,3 +453,11 @@ mapWithKey :: forall k v v'. (k -> v -> v') -> Map k v -> Map k v'
mapWithKey _ Leaf = Leaf
mapWithKey f (Two left k v right) = Two (mapWithKey f left) k (f k v) (mapWithKey f right)
mapWithKey f (Three left k1 v1 mid k2 v2 right) = Three (mapWithKey f left) k1 (f k1 v1) (mapWithKey f mid) k2 (f k2 v2) (mapWithKey f right)

-- | Divide into two maps of keys less and greater than the provided argument.
split :: forall k v. Ord k => k -> Map k v -> { less :: Map k v, greater :: Map k v }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the right be "greater or equal"? Maybe lt and ge would be good label names.

where
mapify {init: ls, rest: gs} =
{less: fromFoldable ls,
greater: fromFoldable $ LL.dropWhile (\(Tuple k' _) -> k' == k) gs}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or instead of dropping equal elements, why not add an eq key too?

Copy link
Contributor Author

@matthewleon matthewleon Jun 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes good sense to me. Going to wait on it until I write a proper tree-traversing version, though, which should take us from nlogn to logn :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants