Either combine only use Semigroup[B]

[johnynek]

note combine(a, b) = a is a semigroup. We are basically forcing this "left-semigroup" on the left side.

I don't think this is a great idea at all. I would rather have:

def combine(that: Either[A, B])(implicit sa: Semigroup[A], sb: Semigroup[B]): Either[A, B] =
  (this, that) match {
    case (Right(lb), Right(rb)) => Right(sb.combine(lb, rb))
    case (Left(la), Left(ra)) => Left(sa.combine(la, ra))
    case (Right(_), l) => l
    case (r, Right(_)) => r
  }

[johnynek]

#1034 and #888 are related.

[johnynek]

so, it originally worked this way, I would say with Either we should revert this decision.

If you want a left semigroup, you can always make one:

implicit val leftSg: Semigroup[A] = new Semigroup[A] {
  def combine(a: A, b: A): A = a
}

But if you want to use Either for something other than error propagation, this is pretty terrible.

I have used either for approximation Monoids where there is also an exact algorithm. For small sizes, I use the Right side, but eventually I switch to Left to approximate (say using a Hyperloglog on the Left and a Set on the right). That case can be handled with a custom wrapper type on Either (and indeed must be or have an orphan since you need some logic to convert Right to Left).

But I still think if you want to only keep the left, you should set up a semigroup to do so.

[non]

If we wanted to go this way we could provide something like:

object ThrowableSemigroup extends Semigroup[Throwable] {
  def combine(x: Throwable, y: Throwable): Throwable = x
}

That way this change would be source compatible in almost all cases.

[yilinwei]

I'm not sure about this. I feel that Either doesn't accumulate errors - if I wanted to accumulate errors I would use a Validated.

It also feels strange because then it becomes inconsistent with ap- combine = ap(that) { _ |+| _ } currently, which I think ought to be the default behaviour of combine.

[ceedubs]

@johnynek at this point it seems like it would be pretty disruptive to make this change. Are you okay with closing this issue out? I'm not sure about your specific use-case, but I think that the support for parTraverse has made moving between accumulating and non-accumulating sum types easier for most of my use-cases.

[johnynek]

I am not arguing for accumulating errors. For that I agree Validated works.

As I mentioned, this is for using Either as a generic sum, which it is.

But I guess if we want a generic sum type (like the example I mentioned where we have two representations that we can convert from one to the other but not vice versa) we are on our own.

We should document: Either is exclusively for monadic error accumulation in cats and any other usage of Either is likely to have confusing outcomes.

If you need a generic sum, that does not have monadic error semantics (Either this or that) you should make a new type because you may get confusing results.

[ceedubs]

@johnynek I see what you are saying. I agree that this is a bit unfortunate. When I made the change in #1034, Cats still had its separate Xor type which was meant to be the non-accumulating counterpart to Validated and I think that this change made more sense in that context. 😔