Jugendhackt / haskell-ricochet

(WIP/Experimental) Ricochet implementation as Haskell Library.
GNU General Public License v3.0
22 stars 0 forks source link

Add Network.Ricochet.Channel #52

Closed froozen closed 8 years ago

froozen commented 8 years ago

This PR implements the Channel type as discussed in the proposal. There are two possible modifications I can offer for this:

froozen commented 8 years ago

Another potential problem with this implementation is, that fmap and transform don't call dupChannel. Users might think of a Channel they transformed as a new instance, yielding some trouble and confusion around skipped values etc.

froozen commented 8 years ago

I replaced the (s -> Maybe a) with an APrism' s a. I'm not so happy about the mergePrisms function, but I don't know any better way. I also went ahead and implemented dupSource. Please tell me what you think about it.

photm5 commented 8 years ago

I wrote a shorter mergePrisms:

mergePrisms :: APrism' s a -> APrism' s b -> APrism' s (Either a b)
mergePrisms p q = prism' fromEither toEither
  where fromEither = either (clonePrism p #) (clonePrism q #)
        toEither s = Left <$> s ^? clonePrism p <|> Right <$> s ^? clonePrism q

However, I think there should be a one-line solution for this, since the problem seems very common and abstract-able with lenses. Feel free to use the 3 line version though :)

photm5 commented 8 years ago

(GitHub line comments suck, you cannot use them without proprietary JavaScript.) You wrote:

writeChannel (MkChannel chan p) = writeChan chan . (^. (re . clonePrism $ p))

That’s what the (#) operator is for:

writeChannel (MkChannel chan p) = writeChan chan . (clonePrism p #)
photm5 commented 8 years ago

I also went ahead and implemented dupSource. Please tell me what you think about it.

Wer macht hat Recht :-). (German for: The one who does something is the one who is right.) My opinion does still apply, however I think dupSource will help in the short run, and we can clean up in the long run, as soon as we figure out the type-level-list stuff we discussed.

photm5 commented 8 years ago
  | otherwise     = MkChannel chan r
  where r = mergePrisms p q

I think inlining r wouldn’t do any harm. Everything else does look good.

froozen commented 8 years ago

Well, my name is @shak-mar, not @shakmar...

Whoops...

In general, I'd like to merge #51 first and add tests for Network.Ricochet.Channel.

photm5 commented 8 years ago

Good idea.

photm5 commented 8 years ago

We might want to have a non-blocking readChannel. And, did you actually try the current readChannel implementation? The implementation makes me assume it eats a lot of resources.

sternenseemann commented 8 years ago

I am gonna revisit this topic. Maybe borrowing some code from this branch