Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Export constructors (maybe in Internal module) #11

Open
mitchellwrosen opened this issue May 18, 2016 · 4 comments
Open

Export constructors (maybe in Internal module) #11

mitchellwrosen opened this issue May 18, 2016 · 4 comments

Comments

@mitchellwrosen
Copy link
Contributor

Might be useful to export the constructors of these data structures.

I'd like to implement a version of delete that tells me whether or not anything was deleted, but I don't believe this is possible with the current API:

multiMapDelete :: (Association k v) => v -> k -> Multimap k v -> STM Bool
multiMapDelete v k (Multimap m) =
  Map.focus ms k m
  where
    ms = 
      \case 
        Just s -> 
          do
            success <- setDelete v s
            Set.null s >>= returnDecision success . bool Focus.Keep Focus.Remove
        Nothing ->
          returnDecision False Focus.Keep
      where
        returnDecision b c = return (b, c)

setDelete :: Element e => e -> Set e -> STM Bool
setDelete e = HAMT.focus deleteM' e . hamt

deleteM' :: Monad m => StrategyM m a Bool
deleteM' = fmap pure $ 
  \case
    Nothing -> (False, Remove)
    Just -> (True, Remove)

Or something along those lines. Anyways, to implement this, I need access to the internals.

@nikita-volkov
Copy link
Owner

This might show a use case, which requires exporting some new functionality. E.g., a different "focus" function. This I will support and will merge a PR. If you're willing to do it, it's best to discuss the design first.

I will not support introducing the "Internals" module, since I am strongly opposed to that convention generally. As an alternative to that I would support releasing the internal HAMT datastructure in a separate library, which I even would do at some point either way, but it would require a heavy revision of all the codebase, and I just don't have the time for that right now.

@nikita-volkov
Copy link
Owner

nikita-volkov commented May 19, 2016

Also it seems like what you need is already achievable with the current API:

multiMapDelete :: (Association k v) => v -> k -> Multimap k v -> STM Bool
multiMapDelete =
  Multimap.focus deleteInforming

deleteInforming :: Applicative m => Focus.StrategyM m a Bool
deleteInforming =
  pure . liftA2 (,) isJust (const Focus.Remove)

@mitchellwrosen
Copy link
Contributor Author

Oh, so it is possible after all :)

Thanks!

@mitchellwrosen
Copy link
Contributor Author

And +1 to releasing the HAMT codebase as a separate library, if you find the time. I don't understand it well enough to help out any :)

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

No branches or pull requests

2 participants