5

I have a function in scala which has no return-value (so unit). This function can sometimes fail (if the user provided parameters are not valid). If I were on java, I would simply throw an exception. But on scala (although the same thing is possible), it is suggested to not use exceptions.

I perfectly know how to use Option or Try, but they all only make sense if you have something valid to return.

For example, think of a (imaginary) addPrintJob(printJob: printJob): Unit command which adds a print job to a printer. The job definition could now be invalid and the user should be notified of this.

I see the following two alternatives:

  1. Use exceptions anyway
  2. Return something from the method (like a "print job identifier") and then return a Option/Either/Try of that type. But this means adding a return value just for the sake of error handling.

What are the best practices here?

theomega
  • 31,591
  • 21
  • 89
  • 127
  • 1
    I would either return a print job identifier as you mention, or you could even return a `Try[Unit]`. – marstran Sep 07 '15 at 07:23

3 Answers3

5

You are too deep into FP :-) You want to know whether the method is successful or not - return a Boolean!

Jacob Eckel
  • 1,633
  • 1
  • 13
  • 22
  • Didn't think of that one. The question is if and how one could return an error message. But then we are already close at an exception... – theomega Sep 07 '15 at 07:37
  • Then you are squarely in the area of Option[String] – Jacob Eckel Sep 07 '15 at 07:40
  • With a strange meaning because `None` would then mean that everything is ok and `Some("Message")` would mean an error. – theomega Sep 07 '15 at 07:41
  • 1
    Yep, None would mean No Worries :-) – Jacob Eckel Sep 07 '15 at 07:43
  • 1
    I'd go with @marstran's suggestion: `Try[Unit]` Your error message is contained in the `Failure(Exception)` and it's pretty obvious that nothing (i.e. `Unit`) means success. – jwvh Sep 07 '15 at 07:44
  • On the other hand, since your function is already side-effecting, throwing an exception seems acceptable in the context. – Jacob Eckel Sep 07 '15 at 07:49
3

According to this Throwing exceptions in Scala, what is the "official rule" Throwing exceptions in scala is not advised as because it breaks the control flow. In my opinion you should throw an exception in scala only when something significant has gone wrong and normal flow should not be continued.

For all other cases it generally better to return the status/result of the operation that was performed. scala Option and Either serve this purpose. imho A function which does not return any value is a bad practice.

For the given example of the addPrintJob I would return an job identifier (as suggested by @marstran in comments), if this is not possible the status of addPrintJob.

Community
  • 1
  • 1
shanmuga
  • 4,329
  • 2
  • 21
  • 35
  • Could you please elaborate? "A function which does not return any value is bad practice" (how do you come to this conclusion? A lot of Scala SDK functions do not return any value) Why would you return a job identifier just for the sake of having a result? This was already suggested by me in the question.... – theomega Sep 08 '15 at 05:54
  • In my opinion if other tasks depend on the this function to be executed correctly then it the responsibility of this function to provide mechanism to check for failure. Only those functions on which no other tasks depend on should return `Unit`. e.g: println. – shanmuga Sep 08 '15 at 08:38
0

The problem is that usually when you have to model things for a specific method it is not about having success or failure ( true or false ) or ( 0 or 1 - Unit exit codes wise ) or ( 0 or 1 - true or false interpolation wise ) , but about returning status info and a msg , thus the most simplest technique I use ( whenever code review naysayers/dickheads/besserwissers are not around ) is that

val msg = "unknown error has occurred during ..."
val ret = 1 // defined in the beginning of the method, means "unknown error"
.... // action 
ret = 0 // when you finally succeeded to implement FULLY what THIS method was supposed to to
msg = "" // you could say something like ok , but usually end-users are not interested in your ok msgs , they want the stuff to work ...

at the end always return a tuple

return ( ret , msg ) 

or if you have a data as well ( lets say a spark data frame )

return ( ret , msg , Some(df))

Using return is more obvious, although not required ( for the purists ) ... Now because ret is just a stupid int, you could quickly turn more complex status codes into more complex Enums , objects or whatnot , but the point is that you should not introduce more complexity than it is needed into your code in the beginning , let it grow organically ...

and of course the caller would call like

 ( ret , msg , mayBeDf ) = myFancyFunc(someparam, etc)

Thus exceptions would mean truly error situations and you will avoid messy try catch jungles ... I know this answer WILL GET down-voted , because well there are too much guys from universities with however bright resumes writing whatever brilliant algos and stuff ending-up into the spagetti code we all are sick of and not something as simple as possible but not simpler and of course something that WORKS.

BUT, if you need only ok/nok control flow and chaining, here is bit more elaborated ok,nok example, which does really throw exception, which of course you would have to trap on an upper level , which works for spark:

  /**
    * a not so fancy way of failing asap, on first failing link in the control chain
    * @return true if valid, false if not
    */
  def isValid(): Boolean = {

    val lst = List(
      isValidForEmptyDF() _,
      isValidForFoo() _,
      isValidForBar() _
    )

    !lst.exists(!_()) // and fail asap ...
  }

  def isValidForEmptyDF()(): Boolean = {
    val specsAreMatched: Boolean = true

    try {
      if (df.rdd.isEmpty) {
        msg = "the file: " + uri + " is empty"
        !specsAreMatched
      } else {
        specsAreMatched
      }
    } catch {
      case jle: java.lang.UnsupportedOperationException => {
        msg = msg + jle.getMessage
        return false
      }
      case e: Exception => {
        msg = msg + e.getMessage()
        return false
      }
    }

  }

Disclaimer: my colleague helped me with the fancy functions syntax ...

Yordan Georgiev
  • 5,114
  • 1
  • 56
  • 53