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

Export constructor via .Internal module? #113

Closed
wants to merge 1 commit into from

Conversation

mtolly
Copy link
Contributor

@mtolly mtolly commented Jul 6, 2017

In my app I wrote some custom traversal functions for a segment of a Map: https://github.com/mtolly/onyxite-customs/blob/9d1b210cdcea75ed1b5220cf9e5b39add21f5901/player/src/OnyxMap.purs#L507-L545

These require access to the Leaf/Two/Three constructors, so for now I've been copying and modifying the module inside my app. But if you are open to it, it would be nice to simply have access via an (explicitly unstable) Internal module. Rather than try to split the definitions I simply moved the whole existing Data.Map to Data.Map.Internal, exported Map(..), and then have Data.Map reexport all of Internal except the constructors.

Apologies for the ugly diff + disruption to other PRs. I totally understand if you'd rather not export them, just figured I'd check. Or, let me know if you'd rather just export them from Data.Map.

@hdgarrood
Copy link
Contributor

I'm not keen on exposing the constructors. I think if anything I'd prefer to add some version of those functions to the library to achieve what you want.

If we had something like foldAsc :: forall k v m. Monoid m => Ord k => k -> k -> (k -> v -> m) -> m, defined in a similar way, with a corresponding foldDesc, I think you would be able to implement an acceptable version of these functions. We could also address #71 fairly easily in terms of this function. We could also make the min and max arguments Maybe values so that you don't have to have a bounded traversal if you don't want an upper or lower bound.

Given that, using the following newtype, I think it should be fairly easy to reconstruct your function:

-- is this available in a library already?
newtype EndoA f = EndoA (f Unit)

instance semigroupEndoA :: Apply f => Semigroup (EndoA f) where
  append (EndoA x) (EndoA y) = EndoA (x *> y)

instance monoidEndoA :: Applicative f => Monoid (EndoA f) where
  mempty = EndoA (pure unit)

@mtolly
Copy link
Contributor Author

mtolly commented Jul 6, 2017

Yeah, I considered adding the functions but wasn't sure if they would be useful to anyone else. I think the function you mentioned would work great as long it has the efficient behavior I've implemented regarding the bounds, where if an entire subtree of Two/Three is outside of the bounds the traverser ignores it entirely.

(For context, my app is a rhythm game simulation in Canvas, so each animation frame the draw function calls the bounded traversal function many times to draw only the events in the time window currently being shown on screen, and each Map under consideration may have hundreds or thousands of keys, whereas only a few dozen likely need to be drawn.)

Another general function that would work for me is split/splitLookup. However I don't know how to implement that on the 2-3 trees efficiently while keeping it balanced.

hdgarrood added a commit that referenced this pull request Jul 7, 2017
hdgarrood added a commit that referenced this pull request Jul 7, 2017
@mtolly mtolly closed this Jul 9, 2017
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