Consider smart constructors for `Buffer`

[ocharles]

Currently, Buffer has 6 constructors, but the choice between Bounded/Single and New/Newest is needlessly imposed on the user. I would suggest:

data Buffer a = ...
unbounded :: Buffer a
bounded :: Int -> Buffer a
newest :: Int -> Buffer a
latest :: a -> Buffer a

You could offer single = bounded 1 if you wanted.

If you think user's should have to think about this, then feel free to close this. Just wanted to throw this out as an idea :)

[Gabriella439]

I like this idea. This also discourages users from pattern matching against the Buffer type so that I can add more constructors later if necessary.

[kvanbere]

Maybe don't even publicly export the internals of Buffer at all. I will open a PR.

[Gabriella439]

Alright, but we need to do two separate releases: one with both the internals and smart constructors exported and then remove the internals in the next release. This makes the upgrade path smoother for downstream libraries.

[kvanbere]

I've been reading #14 and I've also been considering the broadcast stuff which we had to split into another module because it was not compatible with the current API which does not allow calling dupTChan. By the time @quchen added all the meta information he wanted, the return tuple was kind of icky.

Maybe we should return a single thing like a Buffer and then provide the user with functions that mock most of the safe interface from stm:

duplicate :: Buffer a -> STM (Buffer a)
write :: a -> STM (Buffer a)
read :: Buffer a -> STM a
fromInput :: MonadIO m => Buffer a -> Producer' a m () -- should these be in STM?
toOutput :: MonadIO m => Buffer a -> Consumer' a m ()
isFull :: Buffer a -> STM Bool

etc

This also means the broadcast stuff can just live inside the regular module as a smart constructor.

It would then make sense to rename fromInput to fromBuffer, and toOutput to toBuffer. I've always found those names confusing anyway, because often it's not idiomatic to split the return object into two differently typed objects.

[Gabriella439]

The reason for splitting the return object into two different types was originally for the purposes of liveness detection. The very first draft of pipes-concurrency actually unified Input and Output into a single Mailbox type, but this caused problems with the garbage collector not detecting that one of the two was no longer live because the Mailbox type would keep references to both of them alive. I split them into two separate types so that the garbage collector would detect liveness correctly.

However, I later discovered that keeping them as two separate fields had some nice benefits. One of those benefits was the Monoid instance for Input and Output, which isn't as useful if you combine them into a single object. So even though the liveness trick doesn't seem to work any longer I'd still like to keep them as separate types just for that reason.

However, I do agree that we should probably make the Input and Output types opaque and migrate the operations that @quchen requested into them instead of adding more fields to the return value of spawn'. This would be a breaking change for libraries that construct Input values by hand instead of through spawn, but I'm not really aware of any that do so in practice this won't be a big deal to implement.

[kvanbere]

Hey, what happens if you give the smart constructors negative values? Maybe we should check for errors or use Word.

[Gabriella439]

They just thread the Ints to newTBQueueIO, so whatever it does if you provide a negative size.