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 FunctorFilter instances for Iterant&Observable #774

Merged
merged 1 commit into from Nov 14, 2018

Conversation

Projects
None yet
3 participants
@oleg-py
Copy link
Collaborator

commented Nov 8, 2018

There's a new typeclass abstracting over filter and collect in cats since 1.3.0. Since we have streams that support these operations, might as well have the instances :)

@codecov

This comment has been minimized.

Copy link

commented Nov 8, 2018

Codecov Report

Merging #774 into master will decrease coverage by 0.05%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #774      +/-   ##
==========================================
- Coverage   90.42%   90.36%   -0.06%     
==========================================
  Files         419      419              
  Lines       11870    11876       +6     
  Branches     2174     2176       +2     
==========================================
- Hits        10733    10732       -1     
- Misses       1137     1144       +7
@alexandru

This comment has been minimized.

Copy link
Member

commented Nov 8, 2018

Nice 👍

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

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@@ -5495,6 +5496,13 @@ object Observable extends ObservableDeprecatedBuilders {
fa.guaranteeCase(e => finalizer(e).completedL)
override def uncancelable[A](fa: Observable[A]): Observable[A] =
fa.uncancelable
override def functor: Functor[Observable] = this
override def mapFilter[A, B](fa: Observable[A])(f: A => Option[B]): Observable[B] =
fa.map(f).collect { case Some(b) => b }

This comment has been minimized.

Copy link
@ipsq

ipsq Dec 5, 2018

Wouldn't fa.collect(Function.unlift(f)) be more efficient and performant? It spares a liftByOperator operation.

This comment has been minimized.

Copy link
@oleg-py

oleg-py Dec 5, 2018

Author Collaborator

@ipsq unlift results in a function that is called twice to completion: once for isDefinedAt, second time for apply, so it's usually not more efficient. And if a function is side-effecting (although it shouldn't be, but such use-cases are important for monix, esp. for observable), it's absolutely undesirable.

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.