Noob question on how best to refactor

Question
1

Hey Guys,

I have three methods that do exactly the same thing but operate on three object different variables. The methods look like this:

def meth1 : List[String] = {
  if (var1.isDefined) return var1.get

  var1 = Some( expensive_filelist_op( "ext1" ) )
  var1.get
}

My bright idea was to refactor like this:

def common_ops( wrapper : MyMutator, ext : String ) : List[String] = {
  if (wrapper.field.isDefined) return wrapper.field.get

  wrapper.field = Some( expensive_filelist_op( ext ) )
  wrapper.field.get
}

case class MyMutator( var field : List[String] )

def meth1 : List[String] = common_ops( MyMutator( var1), "ext1" )
def meth2 : List[String] = common_ops( MyMutator( var2), "ext2" )
def meth3 : List[String] = common_ops( MyMutator( var3), "ext3" )

Of course, this didn’t work and the values of var1, var2, and var3 are being recalculated on each invocation. Any ideas would be appreciated.

2

Is your method only supposed to calculate the value, when it is first accessed? If yes, maybe a lazy val is all you need:

lazy val var1: List[String] = expensive_filelist_op( "ext1" )

The lazy modifier will cause your expensive operation to not be called until var1 is used.

Btw., the reason your implementation recalculates everytime is, that you create your MyMutator containers inside your methods. You’d have to pull these out to some (private) vals.

1 Like
3

Would var1.getOrElse(expensive_filelist_op( "ext1" )) give the result you want?

4

My first advice would be to try not to use vars. But assuming you have good reasons to use vars or that your real use-case is more complex than this example: you can abstract over a var by using 2 function values, get and set.

def common_ops(get: => Option[List[String]], set: Option[List[String]] => Unit, ext : String): List[String] = {
  if (get.isDefined) return get.get

  set(Some(expensive_filelist_op(ext)))
  get.get
}

def meth1 : List[String] = common_ops(var1, var1_=, "ext1")
1 Like
5

Thanks for the detailed response crater2150. You were bang on in both of your answers. I hadn’t fully appreciated how awesome lazy val was:")