Don't Use Return in Scala?

#21

Yes, but the return statement in your example was not an early return, it was at the end of the function, making it completely redundant (in the original non-inlined example). A competent functional programmer would never use the return statement that way in the first place. So it seems a bit disingenuous to me to condemn all return statements on the basis of that example.

#22

makes it a difference when I write, let’s say:
def UpCase(s: String): String = {
s.toUpperCase()
}

or

def UpCase(s: String): String = {
if (s. length == 0)
return s
else
s.toUpperCase()
}
println( UpCase("") )
println( UpCase(“Hallo”) )

In both cases only a simple “HELLO” get printed, without any faults and errors.
I feel return is a remnant of pointers in low-level programming languages(jump back to Main, so to speak); i don’t know. Maybe someone can explain me why to even use return in the first place in scala? :wink:

It reminds me, I am from a perl background. If I would name variables, without decalring them first with the “my” method, it would be no biggie. But if I would begin the script with a “use strict;” then nothing would work.
Maybe it is similar with the return statement. Some people may use it, to have a clearer structure of the code.

#23

quote: I feel return is a remnant of pointers in low-level programming languages(jump back to Main, so to speak); i don’t know. Maybe someone can explain me why to even use return in the first place in scala?

Several examples were provided earlier in this thread of good uses of return.

#24

Early success seems uncommon to me, but anyway here’s my take at simulating early returns in for comprehensions:

object EarlyReturn {
  val db = ""
  def handleTransfer(req: TransferRequest): Either[Err, Result] = {
    val resultNested = for {
      u1 <- req.user1.lookup(db).left.map(Left(_))
      u2 <- req.user2.lookup(db).left.map { e =>
        Right(Result(s"Cannot find ${req.user2}: $e"))
      }
      depositor <- u2.checkAccess(u1).left.map { _ =>
        Right(Result(s"$u2 has not given you deposit access"))
      }
      withdrawal <- u1.withdraw(req.amount).left.map(Left(_))
      conf <- depositor.accept(withdrawal).left.map(Left(_))
    } yield {
      Result(s"You transferred $withdrawal to $u2, confirmation number $conf")
    }
    resultNested.joinLeft
  }
}

case class Err(msg: String)
case class Result(msg: String)
class User() {
  def checkAccess(that: User): Either[Err, User] = ???
  def withdraw(amount: Double): Either[Err, Double] = ???
  def accept(amount: Double): Either[Err, Int] = ???
}
class UserName() {
  def lookup(db: Any): Either[Err, User] = ???
}
case class TransferRequest(user1: UserName, user2: UserName, amount: Double)

Should be correct. I’ll check if I can factor out the .left.map(e => Left(???)) and .left.map(e => Right(???)).

Update: I managed to do so. Here’s the result:

object EarlyReturn {
  val db = ""
  def handleTransfer(req: TransferRequest): Either[Err, Result] = {
    val resultNested = for {
      u1 <- req.user1.lookup(db).nestError
      u2 <- req.user2.lookup(db).earlyResult { e =>
        Result(s"Cannot find ${req.user2}: $e")
      }
      depositor <- u2.checkAccess(u1).earlyResult { _ =>
        Result(s"$u2 has not given you deposit access")
      }
      withdrawal <- u1.withdraw(req.amount).nestError
      conf <- depositor.accept(withdrawal).nestError
    } yield {
      Result(s"You transferred $withdrawal to $u2, confirmation number $conf")
    }
    resultNested.joinLeft
  }

  implicit class LeftNestingEither[A, B](either: Either[A, B]) {
    def nestError: Either[Either[A, Nothing], B] =
      either.left.map(Left(_))

    def earlyResult[C](errorToResult: A => C): Either[Either[Nothing, C], B] =
      either.left.map(a => Right(errorToResult(a)))
  }
}

case class Err(msg: String)
case class Result(msg: String)
class User() {
  def checkAccess(that: User): Either[Err, User] = ???
  def withdraw(amount: Double): Either[Err, Double] = ???
  def accept(amount: Double): Either[Err, Int] = ???
}
class UserName() {
  def lookup(db: Any): Either[Err, User] = ???
}
case class TransferRequest(user1: UserName, user2: UserName, amount: Double)
#25

Huh, I didn’t even realize that Scala has the return keyword. :slight_smile:

Anyway, before I learned Scala, I was for years part of a Java team, and we had the team policy that every method should have only one return statement, at the end of the method. The reason given, that it was too easy to miss the earlier ones.

1 Like
#26

Other puns that haven’t been exploited include “diminishing returns” and “many happy returns.”

3 Likes
#27

quote: “we had the team policy that every method should have only one return statement, at the end of the method. The reason given, that it was too easy to miss the earlier ones.”

This issue hinges on personal opinion and familiarity. For short functions or methods, I avoid early returns, but for longer ones I find them to be clearer and cleaner. A simple return statement at the top of a function or method to dispense with exceptions is clearer to me than adding a nesting level to the entire method – like the tail wagging the dog. You are concerned about missing an early return, but I am concerned about the added complexity of an already complex algorithm due to superfluous extra nesting levels.

#28

This is a decent try, but doesn’t work. u2 has to contain a User, not a Result. You changed from u2 to u1 at checkAccess so didn’t notice. (Also, I still think the early return is clearer even if this worked, which it doesn’t.)

#29

I don’t see how this hinges on personal opinion and familiarity, could you elaborate? What kind of reason would you consider to be more than personal and familiarity?

If you can rely on there being only one return statement, then you have a sure way to trace back, if not, you have to scan the entire method.

#30

Yep, I’ve made a mistake when converting your code. Now I’ve changed u1 to u2 and the code still compiles fine: https://www.ideone.com/yYGTu5

Notice that Either in Scala 2.12 is right-biased, but nestError and earlyReturn modify the left side of Either - i.e. right side of Either stays the same.

#31

I said it “hinges on personal opinion and familiarity” because either way should be acceptable to reasonable people. I certainly respect your opinion, but I must admit that I don’t understand how searching for return statements is any harder than mentally parsing a deeper nesting structure. The first step in trying to understand any function or method should be to search for return statements.

#32

Consider a different scenario. I have a method m (which causes problems by e.g. overloading a different service) that is used within 5 other methods m1 to m5. I want do quickly know under which conditions method m will be invoked. If there are no early returns, but only nesting structures then I can immediately jump to if conditions, match cases, etc at higher nesting levels. OTOH if there are early returns then I have to look for them and I can easily miss them, because explicit returns in Scala are rare (so they aren’t usually expected).

#33

I don’t think that is the first step in programs designed in a functional manner. Of course, that goes back to teams have an agreed upon style. If different people use very different coding styles, you are going to run into problems. People who are using a more functional approach are going to avoid early returns and make it so that the last expression is the value of the function. Those coming from a more imperative style will feel otherwise. If they work on the same project and mix-and-match those styles, it is going to cause a lot of confusion.

#34

Oh, I see what you’re doing.

That’s pretty clunky; the idea that you might return early infects every line of the for-statement. I don’t see how this could be considered a win over the normal deeply-nested version I posted.

(But yes, it’s possible.)

#35

Let’s revisit what you’ve said earlier:

I have:

  • removed arms of control flow
  • made it clear which variables are in scope (all are to the left of <- operator in for comprehension)
  • replaced everything with for comprehension

I don’t see a difference about shadowing. There’s the same number of variables in scope in every approach.

Every line required either nestError or returnEarly, but:

  • in case of nested flatMaps I need flatMap for every nest level (i.e. every Either)
  • in case of magical ? operator I need to insert that operator for every Either - I could rename nestError to ? if that operator is so cool
  • if you forget to do either nestError or returnEarly on some line then resultNested.joinLeft won’t compile, so you’ll immediately know you’ve forgot something

Also, you’ve forgot to wrap results in Right, i.e. there should be return Right(Response(... instead of return Response(...

After renaming nestError to ? the contest would be between hypothetical:

def handleTransfer(req: TransferRequest): Either[Err, Response] = {
  val u1 = req.user1.lookup(db).?
  val u2 = req.user2.lookup(db) match {
    case Left(e) => return Right(Response(s"Cannot find ${req.user2}: $e"))
    case Right(u) => u
  }
  val depositor = u2.checkAccess(u1) match {
    case Left(e) => return Right(Result(s"$u2 has not given you deposit access"))
    case Right(dep) => dep
  }
  val withdrawal = u1.withdraw(req.amount).?
  val conf = depositor.accept(withdrawal).?
  Result(s"You transferred $withdrawal to $u2, confirmation number $conf")
}

and already working:

  def handleTransfer(req: TransferRequest): Either[Err, Result] = {
    for {
      u1 <- req.user1.lookup(db).?
      u2 <- req.user2.lookup(db).earlyResult { e =>
        Result(s"Cannot find ${req.user2}: $e")
      }
      depositor <- u2.checkAccess(u1).earlyResult { _ =>
        Result(s"$u2 has not given you deposit access")
      }
      withdrawal <- u1.withdraw(req.amount).?
      conf <- depositor.accept(withdrawal).?
    } yield {
      Result(s"You transferred $withdrawal to $u2, confirmation number $conf")
    }
  }.joinLeft
1 Like
#36

What about of point of no return?

1 Like
#37

Okay, fair enough, with renaming and a bunch of custom machinery it’s comparable or slightly better for this example as long as you don’t care about performance. (Slightly better because you have to write a macro to get the earlyResult syntax as compact on the early-return side and still have superior performance.)

I don’t really have time enough to come up with an example of the scale that I usually deal with where there are a couple of nested loops (with early termination) and more to test. Maybe sometime I’ll have time.

For now I’ll grant that if you put in the appropriate machinery–and you need the machinery for .?–this example is comparable either way.

#38

Machinery is already presented in this thread. I think inlining should be enough to recover most of the overhead (unless I got inlining wrong) except single extra wrapping in Either and unwrapping it at the end in case of failure or early return. Late (normal) return doesn’t involve any extra wrapping in Either and unwrapping it (joinLeft does just a cast when invoked on Right value).

I don’t really have time enough to come up with an example of the scale that I usually deal with where there are a couple of nested loops (with early termination) and more to test. Maybe sometime I’ll have time.

Loops are another non-functional constructs so they also would require rewriting to functional counterparts for everything to make sense.

#39

It happens I contributed it to tpolecat’s blog post referenced in the OP, and I’m more than pleased to note that the post now bears that title, so the phrase has been duly exploited to good effect.

I recommend everyone reread the post for the update on -Xlint:nonlocal-return , billed as safe as in safe sex or the safety on guns which in movies is always on when they think it’s off and vice-versa.

1 Like
#40

For a number of alternate approaches.

  1. Already mentioned - chain if/else
  2. Use a match
    • The real solution. Create multiple functions. Early returns are a good indicator your method is too complex. While it might be extreme, try writing your methods without “{}”. If, match, even a comprehension can be written w/o method level “{}”. It can take a bit of rethinking to breakup a method like this. But it can be done. Writing single statement methods is a good way to train your brain to think in functional terms.

To protect your implementation use private[this] or move it to the companion object and use protected.