MetaRx icon indicating copy to clipboard operation
MetaRx copied to clipboard

make ReadStateChannel into a monad

Open fkz opened this issue 9 years ago • 4 comments

I. e. add a flatMap/map to ReadStateChannel that also return a ReadStateChannel instead of just a ReadChannel.

I'm not sure if this is the right solution performance-wise because currently cache is used (so get is called each time we use this flatMap when my understanding is right). This can probably be done better with a more "lazy" approach.

Also, because of this overloading, flatMap(x => code) doesn't work anymore because scala is not able to recognize the type of x (because there are two flatMap's), you can see the necessary change in the diff.

I'd be happy to get feedback of the general idea/specific implementation.

fkz avatar Apr 26 '16 16:04 fkz

This seems like a reasonable feature. @tindzk do you have any objections?

darkfrog26 avatar Aug 03 '16 02:08 darkfrog26

Thanks a lot for the pull request and sorry for not looking into this earlier. I absolutely agree with you that ReadStateChannel should be a monad, but there are some problems with the solution provided:

  • asInstanceOf should not be needed
  • map and flatMap may have a considerable overhead

I will think more about it and try to come up with an alternative solution.

On a similar note, I have been thinking about depending on Cats. It would allow us to check that the monad laws are satisfied (see also http://eed3si9n.com/herding-cats/Monad.html).

tindzk avatar Aug 04 '16 15:08 tindzk

removed the asInstanceOf. The other points remain valid though.

fkz avatar Aug 07 '16 17:08 fkz

Using cats would also solve the problem that flatMap (x => code) can be used again since we won't have two flatMap functions anymore.

Checking laws with cats might also be nice; beware that you have to provide Arbitrary and Eq instances for ReadChannel/ReadStateChannel for this though, so you have to be able to generate arbitrary instances and compare equality of them. I did implement this with having a few base variables and then generate channels that depend on them (for the Arbitrary call) and for the equality change these base variables and see if the channels are emitting the same events. But a problem with this is that there are now a lot of channels depending on these base variables (from all the tests) and so the equality check gets slower and slower because all the unrelated channels have too be updated, too.

fkz avatar Aug 07 '16 18:08 fkz