0

I have this method, given a sequence of instruments, it queries a web service to get the old price and the new price. Then it will use a function to see if the price difference is significant. It will return the first instrument with a significant price difference. Now, my problem is it always returns None unless I add an explicit return statement as below.

I understand scala returns the last expression to be evaluated. Is there a way to refactor this code to avoid an explicit return.

In what situations we cannot avoid having explicit return statements?

// without explicit return

def findEquty[A](instruments: Seq[A], change: (Double, Double) => Boolean): Option[A] = {
  for (instrument <- instruments) {
    val oldPrice = getOldPrice(instrument)
    val newPrice = getNewPrice(instrument)
    if (change(oldPrice, newPrice)) Some(instrument)
  }
  None
}

// with explicit return

def findEquty[A](instruments: Seq[A], change: (Double, Double) => Boolean): Option[A] = {
  for (instrument <- instruments) {
    val oldPrice = getOldPrice(instrument)
    val newPrice = getNewPrice(instrument)
    if (change(oldPrice, newPrice)) return Some(instrument)
  }
  None
}
Tzach Zohar
  • 37,442
  • 3
  • 79
  • 85

3 Answers3

3

Your implementation (without the return statement) contains two expressions:

  • The first is the for expression, which has the type Unit: that's the return type of a for expression without yield (it's interpreted as a call to Seq.foreach which returns Unit)
  • The second is simply None

Every code block in Scala evaluates to the last expression in that block; Therefore, in this case, the entire method body necessarily evaluates into None.

Adding the return statement causes the method to return before reaching this expression (under certain circumstances), hence the "correct" result.

You can solve this by using the more concise find, which does exactly what you need - finds the first element matching a given predicate, or None if none found:

def findEquty[A](instruments: Seq[A], change: (Double, Double) => Boolean): Option[A] = {
  instruments.find(instrument => {
    val oldPrice = getOldPrice(instrument)
    val newPrice = getNewPrice(instrument)
    change(oldPrice, newPrice)
  })
}
Tzach Zohar
  • 37,442
  • 3
  • 79
  • 85
2

For loops return a Unit, meaning they are only executed for their side effects, that is why it doesn't return a value unless you do it explicitly.

You should use the find method which a returns an option with the first element matches the given predicate:

def findEquty[A](instruments: Seq[A], change: (Double, Double) => Boolean): Option[A] = 
  instruments.find {
    val oldPrice = getOldPrice(instrument)
    val newPrice = getNewPrice(instrument)
    change(oldPrice, newPrice)
}
Dani
  • 1,012
  • 9
  • 15
1

A general technique is simply defining your operation over the whole collection and using laziness to only do the amount of work necessary to get a result.

You can filter and get the first element of the result. If you use an Iterator, or possibly a Stream, you prevent doing too much work.

instruments.iterator.filter { instrument =>
  val oldPrice = getOldPrice(instrument)
  val newPrice = getNewPrice(instrument)
  change(oldPrice, newPrice)
}.buffered.headOption
Jasper-M
  • 14,966
  • 2
  • 26
  • 37