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

Add Contravariant Observer and Subscriber #759

Merged
merged 10 commits into from Nov 14, 2018

Conversation

Projects
None yet
2 participants
@Avasil
Copy link
Collaborator

commented Nov 1, 2018

Fixes #748

TODO:

To test laws we need to implement Eq and Gen for Observer / Subscriber. So far I don't have any ideas that make sense so I'm issuing PR in case someone does

@codecov

This comment has been minimized.

Copy link

commented Nov 1, 2018

Codecov Report

Merging #759 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #759      +/-   ##
==========================================
+ Coverage   90.45%   90.46%   +<.01%     
==========================================
  Files         419      419              
  Lines       11879    11908      +29     
  Branches     2187     2183       -4     
==========================================
+ Hits        10745    10772      +27     
- Misses       1134     1136       +2

@alexandru alexandru changed the title WIP: Add Contravariant Observer and Subscriber Add Contravariant Observer and Subscriber Nov 6, 2018


private final class ContravariantObserver[A, B](source: Observer[A])(f: B => A) extends Observer[B] {
// For protecting the contract
private[this] var isDone = false

This comment has been minimized.

Copy link
@Avasil

Avasil Nov 6, 2018

Author Collaborator

@alexandru What's the rule whether we need this protection or not? Sometimes I see this sometimes I don't

@alexandru

This comment has been minimized.

Copy link
Member

commented Nov 14, 2018

@Avasil so Observer and Subscriber are kind of dirty. I mean if we are having problems with testing the laws due to difficulty in implementing some equality test, that's probably a good reason to not do it.

@Avasil

This comment has been minimized.

Copy link
Collaborator Author

commented Nov 14, 2018

@alexandru Would you say we can add instances without testing laws in this case or drop the idea? I guess we could also add the contramap without the instance

@alexandru

This comment has been minimized.

Copy link
Member

commented Nov 14, 2018

Yes, add the contramap without the instance.

Avasil added some commits Nov 14, 2018

Merge remote-tracking branch 'origin/subscriberProfunctor' into subsc…
…riberProfunctor

# Conflicts:
#	monix-reactive/shared/src/main/scala/monix/reactive/instances/CatsContravariantForObserver.scala
#	monix-reactive/shared/src/main/scala/monix/reactive/instances/CatsContravariantForSubscriber.scala
@Avasil

This comment has been minimized.

Copy link
Collaborator Author

commented Nov 14, 2018

@alexandru Done! :)

@alexandru alexandru merged commit 735305a into monix:master Nov 14, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@Avasil Avasil deleted the Avasil:subscriberProfunctor branch Jul 18, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.