Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
6 changes: 1 addition & 5 deletions core/common/src/main/scala/net/liftweb/common/Box.scala
Original file line number Diff line number Diff line change
Expand Up @@ -797,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
14 changes: 14 additions & 0 deletions core/common/src/test/scala/net/liftweb/common/BoxSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,14 @@ class BoxSpec extends Specification with ScalaCheck with BoxGenerator {
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
}
Expand Down Expand Up @@ -306,6 +314,9 @@ class BoxSpec extends Specification with ScalaCheck with BoxGenerator {
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
}
Expand Down Expand Up @@ -370,6 +381,9 @@ class BoxSpec extends Specification with ScalaCheck with BoxGenerator {
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)
}
Expand Down