Simplifying Option[Boolean] expression in Scala

Question

I have code like that:

optionBoolean.getOrElse(false) && otherOptionBoolean.getOrElse(false)

And Scalastyle tells me that it can be simplified. How?

Take a look at [this](https://stackoverflow.com/q/36283704/2501279) question. — Guru Stron, Dec 03 '20 at 09:53
you can use `exists` as `otherOptionBoolean.exists(identity)` but I don't think it's simpler than `getOrElse` — Ivan Stanislavciuc, Dec 03 '20 at 10:14
What do you mean by "can be simplified" ? To me, this looks the simplest possible expression for your requirement. Yes, it might be possible to write it in a more concise and "intelligent" manner but I don't see the need or benefit. Scalastyle is not always right. — sarveshseri, Dec 03 '20 at 14:46
I don't mean anything, I'm just wondering what Scalastyle wanted to tell me in this case :) — Ava, Dec 04 '20 at 12:13

Tomer Shetah · Accepted Answer · 2020-12-03T13:02:08.087

4

You can try the following:

Seq(optionBoolean, otherOptionBoolean).forall(_.contains(true))

In Scala 2.13 (it is very similar in prior versions) the forall method is located at IterableOnce, and its implementation is:

def forall(p: A => Boolean): Boolean = {
  var res = true
  val it = iterator
  while (res && it.hasNext) res = p(it.next())
  res
}

Therefore once there is a value that doesn't satisfy the condition, the loop will break, and the rest will not be tested.

Code run at Scastie.

edited Dec 03 '20 at 13:02

answered Dec 03 '20 at 10:06

Tomer Shetah

8,413
7
27
35

This one is the most readable out of all answers but still I wouldn't say it is a simplified version of my solution. It is just a different approach. – Ava Dec 03 '20 at 11:03
@Ava, I think it is simplified, because I do every action once (one `forall` and one `contains`). In your approach, if you want to add more Booleans, you have to repeat `getOrElse` for each off them. In my approach, you just add them to the `Seq`. In other words, I do 2 operations no matter how many Booleans I have, and you do 1 for every Boolean. Having said that, it is the same time efficiency, but I think that mine is more readable. – Tomer Shetah Dec 03 '20 at 11:07
@TomerShetah This code creates a whole new `List`, fills it (two memory allocations), then iterates over it. So it is really not a "simple" solution in term of execution. Much simpler and more efficient to just test each option in turn. – Tim Dec 04 '20 at 14:30
@Tim you are correct in everything you said. It creates a list, and iterates over it. But this list will be gurbage collected once it will be out of scope. I think that if you do a memory benchmark over your program, and you find that this is your bottleneck, probably Scala isn't the language for you. Probably you should write in c++ it something like that. – Tomer Shetah Dec 04 '20 at 15:05
@TomerShetah Just because Scala is less efficient that C++ does not mean that you should throw performance away when there are faster, simpler alternatives. A JIT compiler can do a good job on the simpler code in other answers, but will struggle with the complexity of this answer. – Tim Dec 04 '20 at 16:59
@Tim, I don't think that creating a list of 2 elements have any memory footprint. Moreover, in the worst case both my, and your solution will be o(n). I think that in this situation the code reuse and easier readability is more important than the minor memory footprint. – Tomer Shetah Dec 04 '20 at 18:21
@TomerShetah The problem is that, in my view, this code is less readable and no more re-usable that the original or other solutions, so the lower performance is not worth it. Objectively, it contains more operations than the original and therefore, again in my view, it cannot really be called "simpler". – Tim Dec 05 '20 at 07:50
@Tim, I see the reuse in the fact that I call `contains` only once. The original solution repeats `getOrElse` and yours repeats `contians`. But in my opinion it is just a matter of style for such a simple question. for such a simple code both are valid. The question is what are we doing when the code complicates and we have for example a hundred of optional booleans. – Tomer Shetah Dec 05 '20 at 08:33

score 3 · Answer 2 · answered Dec 03 '20 at 11:55

3

This is perhaps a bit clearer:

optionBoolean.contains(true) && otherOptionBoolean.contains(true)

answered Dec 03 '20 at 11:55

Tim

26,753
2
16
29

score 3 · Answer 3 · answered Dec 03 '20 at 23:30

3

Just to throw another, not-necessarily-better answer on the pile,

optionBoolean == Some(true) && otherOptionBoolean == Some(true)

or even

(optionBoolean, otherOptionBoolean) == (Some(true), Some(true))

answered Dec 03 '20 at 23:30

Seth Tisue

29,985
11
82
149

tentacle · Answer 4 · 2020-12-03T10:30:20.740

What about custom dsl, that let you work with Option[Boolean] like if is was a Boolean? With all the same operators and same behavior.

You can use something like this:

object Booleans {
  def and(fb: Option[Boolean], other: => Option[Boolean]): Option[Boolean] =
    fb.flatMap(b => if (!b) Some(false) else other)

  def or(fb: Option[Boolean], other: => Option[Boolean]): Option[Boolean] =
    fb.flatMap(b => if (b) Some(true) else other)

  def negate(fb: Option[Boolean]): Option[Boolean] =
    fb.map(!_)

  object Implicits {

    implicit class OptionBooleanDecorator(val fb: Option[Boolean]) extends AnyVal {

      def &&(other: => Option[Boolean]): Option[Boolean] =
        and(fb, other)

      def ||(other: => Option[Boolean]): Option[Boolean] =
        or(fb, other)

      def unary_!(): Option[Boolean] = negate(fb)

    }
  }
}

and somewhere in code:

import Booleans.Implicits._

val b1 = Some(true)
val b2 = Some(true)

val b3 = b1 && b2
val b4 = b1 || b2
val b5 = !b1

You can make it work with any other container. This sort of dsl's that extend some type are quite common in Scala world.

edit: As for last part, namely orElse method - this could be also placed into the dsl, but in this case you loose in composability of operations. So I let all methods return an Option.

Though it looks great, I think it's not great providing logical operators `&&`. It's confusing if not knowing that there are implicits behind. Additionally, it's not applicable to the question. You still have to do `(optionBoolean && otherOptionBoolean).getOrElse(false)` — Ivan Stanislavciuc, Dec 03 '20 at 10:12
It's just an idea. You are not actually overriding logical operators. Options never had them in the first place. You are providing logical operators to boxed type. And since they work exactly the same way as with just plain booleans I don't think this will cause much confusion. As for last part, `orElse` - this could be also placed into the dsl, but in this case you loose in composability of operations. — tentacle, Dec 03 '20 at 10:19

Simplifying Option[Boolean] expression in Scala

4 Answers4

Linked