Boolean operators for F[Boolean]

[danilbykov]

This PR provides operators AND, OR, XOR for F[Boolean]. Small example:

val opt1: Option[Boolean] = true.some
val opt2: Option[Boolean] = false.some
val and: Option[Boolean] = opt1 && opt2

It seems cats core doesn't have such functionality. But it can be useful sometimes. What do you think?

[benhutchison]

So this lifts operations like && from Boolean to F[Boolean]..

The problem is, this is a "slippery slope". If, &&, why not + for F[Int]? Why not isBlank for F[String]? People could lift arbitrary methods of the std lib into the F-space.

I dont want to go there in the general case. There is already syntactic support in Scala for working in the F-space (for-expressions).

So, to do it for Boolean there'd need to be a compelling argument that Boolean was so widely useful that it's justified as an exception. That case would start with pointing out examples in OS codebases where these operators could have made code better.

IMO thats the hurdle to progress this PR.

PS I encourage folks to raise issues to discuss API additions before coding them up as PRs. It's normally the motivation for a change, not the code, that ends up the challenge.

[danilbykov]

Most useful operators are && and ||. I added other operators for completeness. For example, & and | are not widely used even without effects.

Let's say we have three F[Int]: fa, fb and fc. We need to find a sum. It is very easy

(fa, fb, fc).mapN(_ + _ + _)

and looks like summing integers without effects.

But operators && and || have short-circuit evaluation. If I have three F[Boolean]: fa, fb and fc and need to evaluate fa && fb && fc then I should write something like that

    fa.flatMap {
      case true =>
        fb.flatMap {
          case true =>
            fc
          case false =>
            false.pure[F]
        }
      case false =>
        false.pure[F]
    }

which doesn't look perfect as previous example.

[danicheg]

My points on this.
It's possible to fold computations in the F-context for Boolean just by adding the needed instance of Monoid[Boolean] in the local context (see the Monoid[Task[A]] instance in the monix, or Monoid[Option[A]] in the cats). There is not a Monoid[Boolean] instance in the cats because of the semantic of evaluation (see https://eed3si9n.com/herding-cats/Monoid.html#Disjunction+and+Conjunction). But you can create your own with needed semantic and use it when required:

import cats.Monoid
import cats.data.NonEmptyList
import cats.syntax.foldable._

implicit val all: Monoid[Boolean] = new Monoid[Boolean]{
  def empty: Boolean = true
  def combine(x: Boolean, y: Boolean): Boolean = x && y
}

NonEmptyList.of(Option(true), Option(true), Option(false)).fold // Some(false)

Or if you need the any semantic:

import cats.Monoid
import cats.data.NonEmptyList
import cats.syntax.foldable._

implicit val any: Monoid[Boolean] = new Monoid[Boolean]{
  def empty: Boolean = false
  def combine(x: Boolean, y: Boolean): Boolean = x || y
}

NonEmptyList.of(Option(true), Option(true), Option(false)).fold // Some(true)

[danilbykov]

As I understand it doesn't implement short-circuit evaluation. Imagine that F is cats.effect.IO and one F[Boolean] is actually access to database or webservice. There is no reason to make three calls to database if first call returns false.

[benhutchison]

FWIW I appreciate your point @danil that short-circuit evaluation is a useful property.

…

On Wed, 3 Nov 2021 at 10:54 pm, Danil ***@***.***> wrote: As I understand it doesn't implement short-circuit evaluation. Imagine that F is cats.effect.IO and one F[Boolean] is actually access to database or webservice. There is no reason to make three calls to database if first call returns false. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#264 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAXJZGEXQUDBI2JRNJB25LUKEPGJANCNFSM5HHGKQNA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[danicheg]

@danilbykov for the short-circuiting, you can use cats.Foldable#existsM and cats.Foldable#forallM.
Even more, with cats.Foldable#existsM and cats.Foldable#forallM you not needed the Monoid[Boolean] instance at all. And it's looking pretty good:

(fa, fb, fc).forallM(identity) // once a `false` result is encountered, no further effects are produced
(fa, fb, fc).existsM(identity) // once a `true` result is encountered, no further effects are produced.

[danilbykov]

OK. Let's close it.

[benhutchison]

I'm still open to accepting a PR for Boolean BTW. While this get the job done:

(fa, fb, fc).forallM(identity) // once a `false` result is encountered, no further effects are produced
(fa, fb, fc).existsM(identity) // once a `true` result is encountered, no further effects are produced.

This reads clearer intent to me:

fa && fb && fc
fa || fb || fc

Given the "benefit" over eg mapN would be the lazy eval, the unit tests would specifically need to verify lazy eval somehow. Doesnt look like they do ATM.