Skip to content

Add biased operators to Ior#1500

Merged
johnynek merged 1 commit into
typelevel:masterfrom
edmundnoble:ior-merge-bias
Jan 3, 2017
Merged

Add biased operators to Ior#1500
johnynek merged 1 commit into
typelevel:masterfrom
edmundnoble:ior-merge-bias

Conversation

@edmundnoble
Copy link
Copy Markdown
Contributor

Ior is sometimes used in a very biased way. These tools make for convenient use in that case.

@codecov-io
Copy link
Copy Markdown

codecov-io commented Dec 14, 2016

Current coverage is 92.12% (diff: 100%)

Merging #1500 into master will increase coverage by 0.03%

@@             master      #1500   diff @@
==========================================
  Files           246        246          
  Lines          3689       3693     +4   
  Methods        3569       3568     -1   
  Messages          0          0          
  Branches        120        125     +5   
==========================================
+ Hits           3397       3402     +5   
+ Misses          292        291     -1   
  Partials          0          0          

Powered by Codecov. Last update 730d521...6735c43

@edmundnoble edmundnoble force-pushed the ior-merge-bias branch 2 times, most recently from 2672a09 to a486d23 Compare December 15, 2016 04:19
@edmundnoble edmundnoble changed the title Add mergeLeft and mergeRight to Ior Add biased operators to Ior Dec 15, 2016
Comment thread core/src/main/scala/cats/data/Ior.scala Outdated
final def merge[AA >: A](implicit ev: B <:< AA, AA: Semigroup[AA]): AA =
fold(identity, ev.apply, (a, b) => AA.combine(a, b))
final def mergeLeft[AA >: A](implicit ev: B <:< AA): AA =
fold(identity, ev.apply, (a, _) => a)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think you need ev.apply here. I think B <:< AA is already a function B => AA so you can pass it directly without allocating another closure.

Although, it would be interesting to know if scalac avoids allocating a closure when you do fn.apply and instead just passes fn.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point, fixed it in merge too.

@johnynek
Copy link
Copy Markdown
Contributor

👍

@johnynek johnynek mentioned this pull request Jan 2, 2017
11 tasks
@travisbrown
Copy link
Copy Markdown
Contributor

👍, looks reasonable to me.

Comment thread core/src/main/scala/cats/data/Ior.scala Outdated
case Ior.Both(a, b) => fab(a, b)
}

final def putLeft[AA >: A](left: AA): AA Ior B =
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

actually, why does this need to be AA >: A and not just AA? Aren't we fully replacing? Same with putRight?

Add put operators, make ev.apply explicit

Add putLeft and putRight tests

Use ev directly instead of eta-expanding

More polymorphism, thanks johnynek
@edmundnoble
Copy link
Copy Markdown
Contributor Author

Thanks @johnynek, fixed.

@johnynek johnynek merged commit 65cbed3 into typelevel:master Jan 3, 2017
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.

4 participants