Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 27 additions & 5 deletions core/common/src/main/scala/net/liftweb/common/Box.scala
Original file line number Diff line number Diff line change
Expand Up @@ -516,6 +516,30 @@ sealed abstract class Box[+A] extends Product with Serializable{
*/
def foreach[U](f: A => U): Unit = {}

/**
* If this box is a `Failure`, returns a `Failure` box that results from passing this box
* to `fn`. Otherwise, returns this box. Used for transforming Failures without having to
* cover all three cases of Box, which is required by `map`.
*
* `map` is only applied if the box is full, `mapFailure` is only applied if this box is a `Failure`.
*
* @example {{{
* // Returns `Failure("Changed failure")` because this box is a failure.
* Failure("Original Failure") mapFailure { failed => Failure("Changed failure") }
*
* // Returns Full("some-value") on which it was called
* Full("some-value") mapFailure { failure => Failure("Changed failure") }
*
* // Returns this Empty instance
* Empty mapFailure { case value => value }
* }}}
*
* @param fn the function that will be used to transform the Failure if this box is a failure.
* @return A `Failure` instance that results from applying `fn` if this box is a `Failure`, otherwise
* the same instance on which it was called.
*/
def mapFailure(fn: Failure => Failure): Box[A] = this

/**
* If this box is `Full` and contains an object of type `B`, returns a `Full`
* of type `Box[B]`. Otherwise, returns `Empty`.
Expand Down Expand Up @@ -773,11 +797,7 @@ sealed abstract class Box[+A] extends Product with Serializable{
* If the partial function is defined at the current Box's value, apply the
* partial function.
*/
final def collect[B](pf: PartialFunction[A, B]): Box[B] = {
flatMap(value =>
if (pf.isDefinedAt(value)) Full(pf(value))
else Empty)
}
final def collect[B](pf: PartialFunction[A, B]): Box[B] = filter(pf.isDefinedAt).map(pf)

@Bhashit Bhashit Jun 26, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Shadowfiend thought this was more readable than

final def collect[B](pf: PartialFunction[A, B]): Box[B] = flatMap { value =>
  pf.andThen(Full.apply).applyOrElse(value, _ => Empty)
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do think this version will result in a double-application of the PartialFunction guards/pattern matches (a problem that the previous version in the repo also shared):

@ def slap(in: String) = println(s"Uh-oh ${in}"); false 
defined function slap
@ val booyan: PartialFunction[String, Int] = { case "boom" if slap("jam") => 5; case _ => 8 } 
booyan: PartialFunction[String, Int] = <function1>

@ Full("boom").filter(booyan.isDefinedAt).map(booyan) 
Uh-oh jam
Uh-oh jam
res13: Box[Int] = Full(8)
@ booyan.andThen(Full.apply).applyOrElse("boom", { _: String => Empty }) 
Uh-oh jam
res12: Box[Int] = Full(8)

So while I agree this is slightly more readable, I think you nailed it with the original alternative formulation.


/**
* An alias for `collect`.
Expand Down Expand Up @@ -904,6 +924,8 @@ sealed case class Failure(msg: String, exception: Box[Throwable], chain: Box[Fai

override def map[B](f: A => B): Box[B] = this

override def mapFailure(f: (Failure) => Failure): Box[A] = f(this)

override def flatMap[B](f: A => Box[B]): Box[B] = this

override def isA[B](cls: Class[B]): Box[B] = this
Expand Down
36 changes: 31 additions & 5 deletions core/common/src/test/scala/net/liftweb/common/BoxSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ class BoxSpec extends Specification with ScalaCheck with BoxGenerator {
Full(1) reduceLeft {(x: Int, y: Int) => x + y} must_== 1
}
"be used as an Option" in {
Full(1) orElse Some(2) must_== Some(1)
Empty orElse Some(2) must_== Some(2)
Full(1) orElse Some(2) must beSome(1)
Empty orElse Some(2) must beSome(2)
}
"be implicitly defined from an Option. The openOrThrowException method can be used on an Option for example" in {
Some(1).openOrThrowException("This is a test") must_== 1
Expand Down Expand Up @@ -152,14 +152,26 @@ class BoxSpec extends Specification with ScalaCheck with BoxGenerator {
Full(Empty).flatten must_== Empty
}
}
"define a 'mapFailure' method returning itself." in {
val failure = Failure("new-error", Empty, Empty)
Full(1).mapFailure(_ => failure) must_== Full(1)
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like it's a duplicate of one below?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh wait jk I just remembered how this spec is structured heh.

"define a 'collect' method that takes a PartialFunction to transform its contents" in {
"If the partial-function is defined for the contents of this box, returns a full box containing the result of applying that partial function to this Box's contents" in {
Full("Albus") collect { case "Albus" => "Dumbledore"} must_== Full("Dumbledore")
}
"If the partial-function is not defined for the contents of this box, returns Empty" in {
Full("Hermione") collect { case "Albus" => "Dumbledore"} must beEmpty
}
}
"define an 'elements' method returning an iterator containing its value" in {
Full(1).elements.next must_== 1
}
"define a 'toList' method returning a List containing its value" in {
Full(1).toList must_== List(1)
}
"define a 'toOption' method returning a Some object containing its value" in {
Full(1).toOption must_== Some(1)
Full(1).toOption must beSome(1)
}
"return itself if asked for its status with the operator ?~" in {
Full(1) ?~ "error" must_== Full(1)
Expand Down Expand Up @@ -298,14 +310,21 @@ class BoxSpec extends Specification with ScalaCheck with BoxGenerator {
"define a 'flatten' method returning Empty" in {
Empty.flatten must beEmpty
}
"define a 'mapFailure' method returning itself." in {
val failure = Failure("new-error", Empty, Empty)
Empty.mapFailure(_ => failure) must beEmpty
}
"define a 'collect' method returning Empty" in {
Empty collect { case _ => "Some Value"} must beEmpty
}
"define an 'elements' method returning an empty iterator" in {
Empty.elements.hasNext must beFalse
}
"define a 'toList' method returning Nil" in {
Empty.toList must_== Nil
}
"define a 'toOption' method returning None" in {
Empty.toOption must_== None
Empty.toOption must beNone
}
"return a failure with a message if asked for its status with the operator ?~" in {
Empty ?~ "nothing" must_== Failure("nothing", Empty, Empty)
Expand Down Expand Up @@ -358,7 +377,14 @@ class BoxSpec extends Specification with ScalaCheck with BoxGenerator {
Failure("error", Empty, Empty) flatMap {x: String => Full(x.toString)} must_== Failure("error", Empty, Empty)
Failure("error", Empty, Empty).flatten must_== Failure("error", Empty, Empty)
}
"return a itself when asked for its status with the operator ?~" in {
"define a 'mapFailure' method that transforms it into another Failure instance." in {
val exception = new Exception("transformed")
Failure("error", Empty, Empty) mapFailure { _ => Failure("new-error", Full(exception), Empty) } must_== Failure("new-error", Full(exception), Empty)
}
"define a 'collect' method returning itself" in {
Failure("error", Empty, Empty) collect { case _ => "Some Value"} must_== Failure("error", Empty, Empty)
}
"return itself when asked for its status with the operator ?~" in {
Failure("error", Empty, Empty) ?~ "nothing" must_== Failure("error", Empty, Empty)
}
"create a new failure with a chained message if asked for its status with the operator ?~!" in {
Expand Down