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?

Counterirritant

take a look at Option exists function – Hakodate

Take a look at this question. – Cyprinid

you can use exists as otherOptionBoolean.exists(identity) but I don't think it's simpler than getOrElse – Adrianadriana

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. – Claw

I don't mean anything, I'm just wondering what Scalastyle wanted to tell me in this case :) – Counterirritant

Answer 1

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.

Pierro

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. – Counterirritant

@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. – Pierro

@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. – Xanthe

@Xanthe 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. – Pierro

@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. – Xanthe

@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. – Pierro

@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". – Xanthe

@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. – Pierro

Answer 2

This is perhaps a bit clearer:

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

Xanthe

Answer 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))

Bogtrotter

Answer 4

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.

Baht

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) – Adrianadriana

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. – Baht