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!! ^^
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 Option
s, 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.
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)