Skip to content
Merged
Show file tree
Hide file tree
Changes from 13 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
69 changes: 51 additions & 18 deletions core/common/src/main/scala/net/liftweb/common/Box.scala
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,9 @@ sealed trait BoxTrait {
* @return A `Full` containing the transformed value if
* `pf.isDefinedAt(value)` and `Empty` otherwise.
*/
def apply[InType, OutType](pf: PartialFunction[InType, OutType])(value: InType): Box[OutType] =
if (pf.isDefinedAt(value)) Full(pf(value)) else Empty
def apply[InType, OutType](pf: PartialFunction[InType, OutType])(value: InType): Box[OutType] = {
apply(value)(pf)
}

/**
* Apply the specified `PartialFunction` to the specified `value` and return
Expand All @@ -180,8 +181,9 @@ sealed trait BoxTrait {
* @return A `Full` containing the transformed value if
* `pf.isDefinedAt(value)` and `Empty` otherwise.
*/
def apply[InType, OutType](value: InType)(pf: PartialFunction[InType, OutType]): Box[OutType] =
if (pf.isDefinedAt(value)) Full(pf(value)) else Empty
def apply[InType, OutType](value: InType)(pf: PartialFunction[InType, OutType]): Box[OutType] = {
pf.andThen(Full.apply).applyOrElse(value, (_: InType) => Empty)
}

/**
* This implicit transformation allows one to use a `Box` as an `Iterable` of
Expand Down Expand Up @@ -549,18 +551,18 @@ sealed abstract class Box[+A] extends Product with Serializable{
*/
def asA[B](implicit m: Manifest[B]): Box[B] = Empty

/**
* Return this Box if `Full`, or the specified alternative if it is
* empty. Equivalent to `Option`'s `[[scala.Option.orElse orElse]]`.
*/
def or[B >: A](alternative: => Box[B]): Box[B] = alternative

/**
* Returns an `[[scala.collection.Iterator Iterator]]` over the value
* contained in this `Box`, if any.
*/
def elements: Iterator[A] = Iterator.empty

/**
* Return this Box if `Full`, or the specified alternative if it is
* empty. Equivalent to `Option`'s `[[scala.Option.orElse orElse]]`.
*/
def or[B >: A](alternative: => Box[B]): Box[B] = transform { case _: EmptyBox => alternative }

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.

While this is quite clean, I wonder if we should leave or as-is. The JVM will be able to turbo-inline the simple definitions of or, whereas this definition will probably (though not necessarily) be near-impossible to optimize. Given how common it is to call or, I'm tempted to say we should leave it as-is.

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.

Hmm. Yeah, I didn't think it along those lines. I'll change that back.

@Bhashit Bhashit Sep 11, 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.

Do you think keeping the or abstract in the base Box class will make much of a difference? I mean, that way, there's one implementation in Full and one in EmptyBox. It sort of seems cleaner.

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 think that's fine.


/**
* Get a `java.util.Iterator` from the Box.
*/
Expand Down Expand Up @@ -773,10 +775,8 @@ 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] = flatMap { value =>
Box(value)(pf)
}

/**
Expand All @@ -788,6 +788,43 @@ sealed abstract class Box[+A] extends Product with Serializable{
final def collectFirst[B](pf: PartialFunction[A, B]): Box[B] = {
collect(pf)
}

/**
* Transforms this box using the `transformFn`. If `transformFn` is defined for this box,
* returns the result of applying `transformFn` to it. Otherwise, returns this box unchanged.

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 think we should at a note here that if you are looking to change the value inside a Full box, you should really be using map, and if you're looking to change a Failure or Empty into a Full, you should really be using flip.

There's enough complexity in Box that providing some helpful guides in some of this method-specific documentation starts making sense for me. What do you think?

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.

Yeah, that sounds pretty helpful. Will do so.

*
* @example {{{
*
* // Returns Full("alternative") because the partial function covers the case.
* Full("error") transform { case Full("error") => Full("alternative") }
*
* // Returns Full(1), this Full box unchanged, because the partial function doesn't cover the case.
* Full(1) transform { case Full(2) => Failure("error") }
*
* // Returns this Failure("another-error") unchanged because the partial function doesn't cover the case.
* Failure("another-error") transform { case Failure("error", Empty, Empty) => Full("alternative") }
*
* // Returns Full("alternative") for an Empty box since `partialFn` is defined for Empty
* Empty transform { case Empty => Full("alternative") }
*
* // Returns Empty because the partial function is not defined for Empty
* Empty transform { case Failure("error", Empty, Empty) => Full("alternative") }
*
* }}}
*/
def transform[B >: A](transformFn: PartialFunction[Box[A], Box[B]]): Box[B] = {
transformFn.applyOrElse(this, (thisBox: Box[A]) => thisBox)
}

/**
* Returns a `Full` box containing the results of applying `flipFn` to this box if it is a `Failure`,
* `ParamFailure` or `Empty`. Returns `Empty` if this box is `Full`. In other words, it "flips" the
* full/empty status of this Box.
*/
def flip[B](flipFn: EmptyBox => B): Box[B] = this match {
case e: EmptyBox => Full(flipFn(e))
case _ => 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.

It feels like this should be a PartialFunction[Failure, B] and result in a Box[B], no?

@Bhashit Bhashit Jul 10, 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.

That was my first instinct as well. Almost sent a comment asking about these two signatures. I think I'll go with that one then. Now that you said it, it makes more sense. Convert specific cases of Failure into something else.

}

/**
Expand All @@ -800,8 +837,6 @@ final case class Full[+A](value: A) extends Box[A] {

override def openOr[B >: A](default: => B): B = value

override def or[B >: A](alternative: => Box[B]): Box[B] = this

override def exists(func: A => Boolean): Boolean = func(value)

override def forall(func: A => Boolean): Boolean = func(value)
Expand Down Expand Up @@ -868,8 +903,6 @@ sealed abstract class EmptyBox extends Box[Nothing] with Serializable {

override def openOr[B >: Nothing](default: => B): B = default

override def or[B >: Nothing](alternative: => Box[B]): Box[B] = alternative

override def filter(p: Nothing => Boolean): Box[Nothing] = this

override def ?~(msg: => String): Failure = Failure(msg, Empty, Empty)
Expand Down
78 changes: 73 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,38 @@ class BoxSpec extends Specification with ScalaCheck with BoxGenerator {
Full(Empty).flatten must_== Empty
}
}
"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 a 'transform' method that takes a PartialFunction to transform this box into another box" in {
"If the partial-function is defined for this box, returns the result of applying the partial function to it" in {
Full(404) transform {
case Full(x: Int) if x != 200 => Failure("Server error")
} must_== Failure("Server error")
}
"If the partial-function is not defined for this box, returns itself unchanged" in {
Full("Intended Result") transform {
case _: EmptyBox => Full("Alternative")
case Full("Unexpected Result") => Full("Alternative")
} must_== Full("Intended Result")
}
}
"define a 'flip' method returning Empty" in {
Full(1) flip { _ => "No data found" } mustEqual Empty
}
"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 +322,34 @@ class BoxSpec extends Specification with ScalaCheck with BoxGenerator {
"define a 'flatten' method returning Empty" in {
Empty.flatten must beEmpty
}
"define a 'collect' method returning Empty" in {
Empty collect { case _ => "Some Value" } must beEmpty
}
"define a 'transform' method that takes a PartialFunction to transform this Empty box into another box" in {
"If the partial-function is defined for Empty, returns the result of applying the partial function to it" in {
Empty transform {
case Failure("error", Empty, Empty) => Full("failure-alternative")
case Empty => Full("alternative")
} must_== Full("alternative")
}
"If the partial-function is not defined for Empty, returns Empty" in {
Empty transform { case Failure("The Phantom Menace", Empty, Empty) => Full("Return Of The Jedi") } must_== Empty
}
}
"define a 'flip' method returning a Full box" in {
Empty flip {
case Empty => "flipped-empty"
case _ => "flipped-failure"
} mustEqual Full("flipped-empty")
}
"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 +402,31 @@ 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 'collect' method returning itself" in {
Failure("error", Empty, Empty) collect { case _ => "Some Value" } must_== Failure("error", Empty, Empty)
}
"define a 'transform' method that takes a PartialFunction to transform this Failure into another box" in {
"If the partial-function is defined for this Failure, returns the result of applying the partial function to it" in {
Failure("The Phantom Menace") transform {
case Failure("The Phantom Menace", Empty, Empty) => Full("Return Of The Jedi")
} must_== Full("Return Of The Jedi")

Failure("The Phantom Menace") transform {
case Failure("The Phantom Menace", Empty, Empty) => Failure("Clones")
case _ => Full("Jedi")
} must_== Failure("Clones")
}
"If the partial-function is not defined for this Failure, returns itself unchanged" in {
Failure("Clones") transform { case Failure("The Phantom Menace", Empty, Empty) => Full("Jedi") } must_== Failure("Clones")
}
}
"define a 'flip' method returning a Full box" in {
Failure("error", Empty, Empty) flip {
case Empty => "flipped-empty"
case _: Failure => "flipped-failure"
} must_== Full("flipped-failure")
}
"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