(Scala) Am I using Options correctly?

2019-08-17 04:23发布

问题:

I'm currently working on my functional programming - I am fairly new to it. Am i using Options correctly here? I feel pretty insecure on my skills currently. I want my code to be as safe as possible - Can any one point out what am I doing wrong here or is it not that bad? My code is pretty straight forward here:

def main(args: Array[String]): Unit =
  {
    val file = "myFile.txt"
    val myGame = Game(file) //I have my game that returns an Option here

    if(myGame.isDefined) //Check if I indeed past a .txt file
    {
      val solutions = myGame.get.getAllSolutions() //This returns options as well
      if(solutions.isDefined) //Is it possible to solve the puzzle(crossword)
      {
        for(i <- solutions.get){ //print all solutions to the crossword
          i.solvedCrossword foreach println
        }
      }
    }
  }

-Thanks!! ^^

回答1:

As a rule of thumb, you can think of Option as a replacement for Java's null pointer. That is, in cases where you might want to use null in Java, it often makes sense to use Option in Scala.

Your Game() function uses None to represent errors. So you're not really using it as a replacement for null (at least I'd consider it poor practice for an equivalent Java method to return null there instead of throwing an exception), but as a replacement for exceptions. That's not a good use of Option because it loses error information: you can no longer differentiate between the file not existing, the file being in the wrong format or other types of errors.

Instead you should use Either. Either consists of the cases Left and Right where Right is like Option's Some, but Left differs from None in that it also takes an argument. Here that argument can be used to store information about the error. So you can create a case class containing the possible types of errors and use that as an argument to Left. Or, if you never need to handle the errors differently, but just present them to the user, you can use a string with the error message as the argument to Left instead of case classes.

In getAllSolutions you're just using None as a replacement for the empty list. That's unnecessary because the empty list needs no replacement. It's perfectly fine to just return an empty list when there are no solutions.


When it comes to interacting with the Options, you're using isDefined + get, which is a bit of an anti pattern. get can be used as a shortcut if you know that the option you have is never None, but should generally be avoided. isDefined should generally only be used in situations where you need to know whether an option contains a value, but don't need to know the value.

In cases where you need to know both whether there is a value and what that value is, you should either use pattern matching or one of Option's higher-order functions, such as map, flatMap, getOrElse (which is kind of a higher-order function if you squint a bit and consider by-name arguments as kind-of like functions). For cases where you want to do something with the value if there is one and do nothing otherwise, you can use foreach (or equivalently a for loop), but note that you really shouldn't do nothing in the error case here. You should tell the user about the error instead.



回答2:

  1. When using Option, it is recommended to use match case instead of calling 'isDefined' and 'get'
  2. Instead of the java style for loop, use higher-order function:

    myGame match {
      case Some(allSolutions) =>
        val solutions = allSolutions.getAllSolutions
        solutions.foreach(_.solvedCrossword.foreach(println))
      case None =>
    }
    


回答3:

If all you need here is to print it in case all is good, you can use for-comprehension which is considered quite idiomatic Scala way

    for {
      myGame <- Game("mFile.txt")
      solutions <- myGame.getAllSolutions()
      solution <- solutions
      crossword <- solution.solvedCrossword
    } println(crossword)


标签: scala types