Skip to main content
Code Review

Return to Answer

added 1073 characters in body
Source Link
kiritsuku
kiritsuku
  • 1.6k
  • 1
  • 12
  • 14

EDIT

To address your edit, it seems that you never know the type you can get. This will complicate things a lot since you have to check the type of each element explicitly. I don't think it is possible to do this in a clean way without the use of a library which can do the typechecks for you. I suggest to take a look at some Scalaz or Shapeless code.

Nevertheless is is far easier to do such type checks in the parser and not in the AST traverser. Thus, I suggest using a JSON parser which can handle a type format given by the user and returns an error if it finds some unexpected content. I don't know if Play! can parse things like the following (which can be done in lift-json):

val json = """{ "foo": [1, 2, 3], "bar": [2, 5] }"""
class Content(foo: List[Double], bar: List[Double])
parse(json).extract[Content]

EDIT

To address your edit, it seems that you never know the type you can get. This will complicate things a lot since you have to check the type of each element explicitly. I don't think it is possible to do this in a clean way without the use of a library which can do the typechecks for you. I suggest to take a look at some Scalaz or Shapeless code.

Nevertheless is is far easier to do such type checks in the parser and not in the AST traverser. Thus, I suggest using a JSON parser which can handle a type format given by the user and returns an error if it finds some unexpected content. I don't know if Play! can parse things like the following (which can be done in lift-json):

val json = """{ "foo": [1, 2, 3], "bar": [2, 5] }"""
class Content(foo: List[Double], bar: List[Double])
parse(json).extract[Content]
added 104 characters in body
Source Link
kiritsuku
kiritsuku
  • 1.6k
  • 1
  • 12
  • 14

Nevertheless, Try should only be used when you have to work with exceptions. When there is no special reason, theyexceptions should be avoided - they are not here to control the control flow but to tell us that an error occurred which normally can not be handled. They are some runtime thing - and why to use runtime things in a statically typed language? When you use monads the compiler always enforces you to write correct code:

scala> def countLen(xs: List[String]) = SomeOption(xs) collect { case List(str) => str } map (_.length)
countLen: (xs: List[String])Option[Int]

scala> countLen(List("hello"))
res8: Option[Int] = Some(5)

scala> countLen(Nil)
res9: Option[Int] = None 

scala> countLen(null) // please, never use null in Scala
res13: Option[Int] = None

With collect you ensure that you don't throw an exception during matching the contents. After that with map one can operate on the String and doesn't care anymore if an empty or a full list is passed to countLen.

Nevertheless, Try should only be used when you have to work with exceptions. When there is no special reason, they should be avoided - they are not here to control the control flow but to tell us that an error occurred which normally can not be handled. They are some runtime thing - and why to use runtime things in a statically typed language? When you use monads the compiler always enforces you to write correct code:

scala> def countLen(xs: List[String]) = Some(xs) collect { case List(str) => str } map (_.length)
countLen: (xs: List[String])Option[Int]

scala> countLen(List("hello"))
res8: Option[Int] = Some(5)

scala> countLen(Nil)
res9: Option[Int] = None

With collect you ensure that you don't throw an exception during matching the contents. After that with map one can operate on the String and doesn't care anymore if an empty or full list is passed to countLen.

Nevertheless, Try should only used when you have to work with exceptions. When there is no special reason, exceptions should be avoided - they are not here to control the control flow but to tell us that an error occurred which normally can not be handled. They are some runtime thing - and why to use runtime things in a statically typed language? When you use monads the compiler always enforces you to write correct code:

scala> def countLen(xs: List[String]) = Option(xs) collect { case List(str) => str } map (_.length)
countLen: (xs: List[String])Option[Int]

scala> countLen(List("hello"))
res8: Option[Int] = Some(5)

scala> countLen(Nil)
res9: Option[Int] = None 

scala> countLen(null) // please, never use null in Scala
res13: Option[Int] = None

With collect you ensure that you don't throw an exception during matching the contents. After that with map one can operate on the String and doesn't care anymore if an empty or a full list is passed to countLen.

Source Link
kiritsuku
kiritsuku
  • 1.6k
  • 1
  • 12
  • 14

Having monads in your data structures is not messy, they allow you always to flatten their content during nested calculation steps. A simple definition of them can be:

trait Monad[A] {
  def map[B](f: A => B): Monad[B]
  def flatMap[B](f: A => Monad[B]): Monad[B]
}

Because Option is a monad you do not have to work with nested types:

scala> def inc(i: Int) = Option(i+1)
inc: (i: Int)Option[Int]

scala> val opt = inc(0)
opt: Option[Int] = Some(1)

scala> val nested = opt map inc
nested: Option[Option[Int]] = Some(Some(2))

scala> val flattened = opt flatMap inc
flattened: Option[Int] = Some(2)

Thus, there should never be a reason to nest monads deeply. Scala also has the for-comprehension which automatically decomposes monads:

scala> :paste
// Entering paste mode (ctrl-D to finish)

val none = for {
  o1 <- inc(0)
  o2 <- inc(o1)
  o3 <- inc(o2)
} yield o3

val some = for {
  o1 <- inc(0)
  o2 <- inc(o1)
  o3 <- inc(o2-1)
} yield o3

// Exiting paste mode, now interpreting.

none: Option[Int] = None
some: Option[Int] = Some(2)

In order to work with exceptions scala.util.Try is introduced in 2.10:

scala> import scala.util.{Try, Success, Failure}
import scala.util.{Try, Success, Failure}

scala> def err: Int = throw new RuntimeException
err: Int

scala> val fail = Try{err} map (_+1)
fail: scala.util.Try[Int] = Failure(java.lang.RuntimeException)

scala> val succ = Try{0} map (_+1)
succ: scala.util.Try[Int] = Success(1)

Nevertheless, Try should only be used when you have to work with exceptions. When there is no special reason, they should be avoided - they are not here to control the control flow but to tell us that an error occurred which normally can not be handled. They are some runtime thing - and why to use runtime things in a statically typed language? When you use monads the compiler always enforces you to write correct code:

scala> def countLen(xs: List[String]) = Some(xs) collect { case List(str) => str } map (_.length)
countLen: (xs: List[String])Option[Int]

scala> countLen(List("hello"))
res8: Option[Int] = Some(5)

scala> countLen(Nil)
res9: Option[Int] = None

With collect you ensure that you don't throw an exception during matching the contents. After that with map one can operate on the String and doesn't care anymore if an empty or full list is passed to countLen.

Now, look at the documentation of Option, there are more useful methods which allow safe error handling. The only thing to keep in mind is not to use method get or a pattern match on the monad during calculation:

scala> Some(3).get
res10: Int = 3

scala> None.get // this throws an exception
java.util.NoSuchElementException: None.get

// this looks ugly and does not safe you from anything because it is equal
// to a null check
anyOption match {
  case Some(e) => // no error
  case None => // error
}

Now you may ask how to get the contents of a monad? That is a good question and the answer is: you won't. When you use a monad you say something like: Hey, I'm not interested if my code threw an error or if all worked fine. I want that my code works up to end and then I will look what's happened. Pattern matching on the content of a monad (means: accessing their content explicitly) is the last thing which should be done and only if there is no other way any more to go further with control flow.

There a lot of monads available in Scala and you have the possibility to easily create your own. Option allows only to get a notification if something happened wrong. If you wanna have the exact error message you can use Try (for exceptions) or Either (for all other things). Because Either is not really a monad (it has a Left- and a RightProjection which are the monads) it is unhandy to use. Thus, if you want to effectively work with error messages or even stack them you should take a look at scalaz.Validation.