How do you stop creating the [Collection] option after reaching the first No?

Question

When creating a collection inside Option each attempt to make the next member of the collection may fail, which will also lead to the failure of the collection as a whole. After the first refusal to make a member, I would like to immediately refuse and return None for the entire collection. What is the idiomatic way to do this in Scala?

Here is one approach I came up with:

 def findPartByName(name: String): Option[Part] = . . . def allParts(names: Seq[String]): Option[Seq[Part]] = names.foldLeft(Some(Seq.empty): Option[Seq[Part]]) { (result, name) => result match { case Some(parts) => findPartByName(name) flatMap { part => Some(parts :+ part) } case None => None } } 

In other words, if any findPartByName call returns None , allParts returns None . Otherwise, allParts returns a Some containing a collection of Parts , all of which are guaranteed. The empty collection is fine.

The above has the advantage that after the first failure it stops calling findPartByName . But foldLeft once for each name, independently.

Here is the version that is reset as soon as findPartByName returns None :

 def allParts2(names: Seq[String]): Option[Seq[Part]] = Some( for (name <- names) yield findPartByName(name) match { case Some(part) => part case None => return None } ) 

Currently, I believe that the second version is more readable, but (a) what seems to be the most readable is likely to change, as I gain more experience with Scala, (b) I get the impression that early return disapproving of Scala and (c) none of them does what is happening especially obvious to me.

The combination of "all or nothing" and "rejection of the first rejection" seems like such a basic programming concept, I believe that for its expression there should be a common Scala or functional idiom.

Answer 1

return in your code is actually a couple of levels in anonymous functions. As a result, it should be implemented by eliminating an exception that falls into an external function. It is ineffective or beautiful, so frown.

It is easiest and most efficient to write this using the while and Iterator .

 def allParts3(names: Seq[String]): Option[Seq[Part]] = { val iterator = names.iterator var accum = List.empty[Part] while (iterator.hasNext) { findPartByName(iterator.next) match { case Some(part) => accum +:= part case None => return None } } Some(accum.reverse) } 

Since we do not know what these Seq names , we should create an iterator for its efficient loop, and not using tail or indexes. The while loop can be replaced with a recursive internal function, but with an iterator, the while is clearer.

Answer 2

Scala collections have some opportunities to use laziness to achieve this.

You can use view and takeWhile :

 def allPartsWithView(names: Seq[String]): Option[Seq[Part]] = { val successes = names.view.map(findPartByName) .takeWhile(!_.isEmpty) .map(_.get) .force if (!names.isDefinedAt(successes.size)) Some(successes) else None } 

Using ifDefinedAt avoids the possibility of transferring a long input of names in case of an early failure.

You can also use toStream and span to achieve the same:

 def allPartsWithStream(names: Seq[String]): Option[Seq[Part]] = { val (good, bad) = names.toStream.map(findPartByName) .span(!_.isEmpty) if (bad.isEmpty) Some(good.map(_.get).toList) else None } 

I found an attempt to mix view and span forces findPartByName be evaluated twice per item if successful.

The whole idea of ​​returning the error condition, if any error occurs, however, is more like work ("job?") On throwing and throwing exceptions. I suppose it depends on the context in your program.

Answer 3

Combine other answers, that is, a mutable flag with a map and accept, if we love.

Given an endless stream:

 scala> var count = 0 count: Int = 0 scala> val vs = Stream continually { println(s"Compute $count") ; count += 1 ; count } Compute 0 vs: scala.collection.immutable.Stream[Int] = Stream(1, ?) 

Take until the predicate works:

 scala> var failed = false failed: Boolean = false scala> vs map { case x if x < 5 => println(s"Yup $x"); Some(x) case x => println(s"Nope $x"); failed = true; None } takeWhile (_.nonEmpty) map (_.get) Yup 1 res0: scala.collection.immutable.Stream[Int] = Stream(1, ?) scala> .toList Compute 1 Yup 2 Compute 2 Yup 3 Compute 3 Yup 4 Compute 4 Nope 5 res1: List[Int] = List(1, 2, 3, 4) 

or more simply:

 scala> var count = 0 count: Int = 0 scala> val vs = Stream continually { println(s"Compute $count") ; count += 1 ; count } Compute 0 vs: scala.collection.immutable.Stream[Int] = Stream(1, ?) scala> var failed = false failed: Boolean = false scala> vs map { case x if x < 5 => println(s"Yup $x"); x case x => println(s"Nope $x"); failed = true; -1 } takeWhile (_ => !failed) Yup 1 res3: scala.collection.immutable.Stream[Int] = Stream(1, ?) scala> .toList Compute 1 Yup 2 Compute 2 Yup 3 Compute 3 Yup 4 Compute 4 Nope 5 res4: List[Int] = List(1, 2, 3, 4) 

Answer 4

I think your allParts2 function has a problem, since one of the two branches of your match operator will have a side effect. The return is a non-idiomatic bit that behaves as if you were performing an imperative transition.

The first function looks better, but if you are interested in the non-optimal iteration that foldLeft can create, you should probably go for a recursive solution, as shown below:

 def allParts(names: Seq[String]): Option[Seq[Part]] = { @tailrec def allPartsRec(names: Seq[String], acc: Seq[String]): Option[Seq[String]] = names match { case Seq(x, xs@ _*) => findPartByName(x) match { case Some(part) => allPartsRec(xs, acc +: part) case None => None } case _ => Some(acc) } allPartsRec(names, Seq.empty) } 

I did not compile / run it, but the idea should be there, and I think this is more idiomatic than using the reverse trick!

Answer 5

I keep thinking that it should be single or double line. I came up with one:

 def allParts4(names: Seq[String]): Option[Seq[Part]] = Some( names.map(findPartByName(_) getOrElse { return None }) ) 

Advantage:

• The goal is very clear. There is no mess and no exotic or non-standard Scala.

Disadvantages:

• Early return violates referential transparency, as Aldo Strekkadanio noted. You cannot put the body of allParts4 in your calling code without changing its value.

• Perhaps this is ineffective due to an internal throw and an exception trap, as the winged gentleman pointed out.

Of course, I put this in some real code, and ten minutes later I included the expression inside something else and, as predicted, got amazing behavior. So now I understand a little better why early on return disapproving.

This is such a general operation, which is important in the code that Option uses a lot, and Scala is usually so good at combining things, I can’t believe that there isn’t a pretty natural idiom that’s right.

Are monads not suitable for determining how to combine actions? Is there a GiveUpAtTheFirstSignOfResistance monad?