nikita-volkov / stm-containers

Containers for STM
http://hackage.haskell.org/package/stm-containers
MIT License
66 stars 13 forks source link

Export constructors (maybe in Internal module) #11

Open mitchellwrosen opened 8 years ago

mitchellwrosen commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

Oh, so it is possible after all :)

Thanks!

mitchellwrosen commented 8 years ago

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 :)