Scala style: how far to nest functions?

Question

One of the advantages of Scala is that it gives you great control over scope. You can nest functions like this:

def fn1 = {
  def fn11 = {
    ...
  }
  def fn12 = {
    ...
    def fn121 = {
      ...
    }
  }
  ...
  def fn13 = {
    ...
  }
}

The problem here is that fn1 may start to look a bit intimidating. Coming from a Java background, we are advised to keep functions small enough to be viewed on a single "page" in the IDE.

What would you think about taking fn12 out of fn1 based on the reasoning: "It's only used in fn1 right now, but it might come in useful somewhere else in the class later on..."

Also, would you have a preference as to where to place the nested functions - before or after the code that calls them?

Answer 1

In general I don't see that much nesting of functions in real code. It runs against the ethos of keeping methods simple and concise. Such nesting is mainly useful for closures where you'll be using some of the parameters from the outer scope (e.g. the inner loop of a recursive function), so it's cleaner than declaring it outside and having to re-pass those arguments explicitly.

You have to place the nested functions before the code that calls them or it's a forward reference and won't compile. (In objects / classes you can place them after, but not in methods.)

Answer 2

There are a few patterns that take advantage of one layer of nesting.

Recursion, where it is used to hide implementation details (and is cleaner than separating into two separate top-level methods):

def callsRecursive(p: Param): Result = {
  def recursive(p: Param, o: OtherParam, pr: PartialResult = default): Result = {
    ...
  }
}

Scope-safe don't-repeat-yourself:

def doesGraphics(p: Point) {
  def up(p: Point): Point = // ...
  def dn(p: Point): Point = // ...
  def lf(p: Point): Point = // ...
  def rt(p: Point): Point = // ...
  /* Lots of turtle-style drawing */
}

And more esoteric tricks like shadowing implicit conversions for a local block.

If you need both of these, I could envision nesting twice. More than that is likely overkill, mostly because you are probably making one method do too much. You should think about how to subdivide the problem with clean interfaces that can then become their own methods, rather than having a messy hodge-podge of closures around all sorts of variables defined within the method. Big methods are like global variables: everything becomes too dependent on the details of implementation and too hard to keep track of. If you're ready to do the appropriate amount of thinking to make something have a decent interface, even if you only need it once, then consider taking it out to the top level. If you aren't going to think that hard, my inclination is to leave it inside to avoid polluting the interface.

In any case, don't be afraid to create a method anywhere you need it. For example, suppose you find yourself deep within some method with two collections each of which have to have the same operation performed on them at specific points in the logic. Don't worry if you're one or two or three methods deep! Just create the method where it's needed, and call it instead of repeating yourself. (Just keep in mind that creating a list and mapping is an alternative if you simply need to process several things at the same place.)

Answer 3

If you have a top level function like the one you describe it is probably doing to much.

TDD helps as well in the decision if this is the case: Is still everything easily testable.

If I come to the conclusion that this is actually the case I refactor to get the inner functions out as dependencies, with their own tests.

In the end result I make very limited use of functions defined in functions defined ... I also put a much stricter limit on method size: about 10-15 lines in java, even less in scala, since it less verbose.

I put internal functions mostly at the top of the outer method, but it hardly matters since its so short anyway.

Answer 4

I consider it as a best practice to always use the lowest visibility. If a nested function is needed for a different function, it could be moved anyway.