Skip to content

Add Contravariant Observer and Subscriber#759

Merged
alexandru merged 10 commits into
monix:masterfrom
Avasil:subscriberProfunctor
Nov 14, 2018
Merged

Add Contravariant Observer and Subscriber#759
alexandru merged 10 commits into
monix:masterfrom
Avasil:subscriberProfunctor

Conversation

@Avasil
Copy link
Copy Markdown
Collaborator

@Avasil Avasil 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
Copy link
Copy Markdown

codecov Bot 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
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@alexandru
Copy link
Copy Markdown
Member

@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
Copy link
Copy Markdown
Collaborator Author

Avasil 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
Copy link
Copy Markdown
Member

Yes, add the contramap without the instance.

…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
Copy link
Copy Markdown
Collaborator Author

Avasil commented Nov 14, 2018

@alexandru Done! :)

@alexandru alexandru merged commit 735305a into monix:master Nov 14, 2018
@Avasil Avasil deleted the subscriberProfunctor branch July 18, 2019 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants