CODEWITHSUNDEEP
f#
Scott Nimrod
Scott Nimrod

Reputation: 11595

When should I use a function within a function versus a separate private function?

When should I use a function within a function versus a separate private function?

I observed that a function that I wrote was fairly long:

let optionsFor piece (positions:Space list) =

    let yDirection = match piece with
                     | Black _ -> -1
                     | Red   _ ->  1

    let sourceX , sourceY = 
        match piece with
        | Black (checker , pos) -> pos
        | Red   (checker , pos) -> pos

    let optionsForPiece = 
        (fun pos -> pos = ((sourceX - 1) , (sourceY + yDirection)) ||
                    pos = ((sourceX + 1) , (sourceY + yDirection)))

    let availableSelection = 
        (fun space -> match space with
                      | Available pos -> Some pos
                      | Allocated _   -> None)

    let availablePositions = 
        positions |> List.filter toAvailable
                  |> List.choose availableSelection

    availablePositions |> List.filter optionsForPiece

Thus, I considered refactoring the function above into several small functions.

However, I am not sure if this is necessary in functional programming.

What is the current recommendation on inner functions versus extracting them out to private functions?

Appendix:

open NUnit.Framework
open FsUnit

(* Types *)
type Black = BlackKing | BlackSoldier
type Red =   RedKing   | RedSoldier

type Coordinate = int * int

type Piece =
    | Black of Black * Coordinate
    | Red   of Red   * Coordinate

type Space =
    | Allocated of Piece
    | Available of Coordinate

type Status =
    | BlacksTurn | RedsTurn
    | BlackWins  | RedWins

(* Functions *)
let black coordinate = Allocated (Black (BlackSoldier , coordinate))
let red   coordinate = Allocated (Red   (RedSoldier   , coordinate))

let startGame () =
    [ red (0,0);  red (2,0);  red (4,0);  red (6,0)
      red (1,1);  red (3,1);  red (5,1);  red (7,1)
      red (0,2);  red (2,2);  red (4,2);  red (6,2)

      Available (1,3); Available (3,3); Available (5,3); Available (7,3)
      Available (0,4); Available (2,4); Available (4,4); Available (6,4)

      black (1,5);  black (3,5);  black (5,5);  black (7,5)
      black (0,6);  black (2,6);  black (4,6);  black (6,6)
      black (1,7);  black (3,7);  black (5,7);  black (7,7) ] , BlacksTurn

let private toAvailable = 
    (fun space -> match space with
                  | Available pos -> true
                  | _             -> false)

let available (positions:Space list) = positions |> List.filter toAvailable

let optionsFor piece (positions:Space list) =

    let yDirection = match piece with
                     | Black _ -> -1
                     | Red   _ ->  1

    let sourceX , sourceY = 
        match piece with
        | Black (checker , pos) -> pos
        | Red   (checker , pos) -> pos

    let optionsForPiece = 
        (fun pos -> pos = ((sourceX - 1) , (sourceY + yDirection)) ||
                    pos = ((sourceX + 1) , (sourceY + yDirection)))

    let availableSelection = 
        (fun space -> match space with
                      | Available pos -> Some pos
                      | Allocated _   -> None)

    let availablePositions = 
        positions |> List.filter toAvailable
                  |> List.choose availableSelection

    availablePositions |> List.filter optionsForPiece

Upvotes: 0

Views: 81

Answers (1)

rmunn
rmunn

Reputation: 36698

This is more opinion-based, but I'll offer my opinion.

My rule of thumb would be that if the "helper" function is tighly associated with the "main" function, I'd write it as a nested function. If they're not tightly associated, I'd write the helper function as a separate function -- and I might not even make it private, because you never know when it might come in handy for other code in a different module.

An example of a tightly associated inner function would be the kind of loop-with-accumulator function that you often end up writing in recursive functional programming. For example, here's some code I wrote for an F# programming exercise:

module BinarySearchTree

type Node<'T> =
    { left: Node<'T> option
      value: 'T
      right: Node<'T> option }

let singleton v = { left = None; value = v; right = None }

let rec insert v t =
    if v <= t.value
        then match t.left with
             | None -> { t with left = singleton v |> Some }
             | Some n -> { t with left = insert v n |> Some }
        else match t.right with
             | None -> { t with right = singleton v |> Some }
             | Some n -> { t with right = insert v n |> Some }

let fromList l =
    match l with
    | [] -> failwith "Can't create a tree from an empty list"
    | hd::tl ->
        tl |> List.fold (fun t v -> insert v t) (singleton hd)

let toList t =
    let rec loop acc = function
        | None -> acc
        | Some node ->
            (loop [] node.left) @ (node.value :: (loop [] node.right))
    loop [] (Some t)

Take a look at that last toList function. It has an inner function that I called loop, which would make no sense as a standalone function. It is so tightly associated to the toList function that it just makes sense to keep it as an inner function, not accessible from outside toList.

However, when I wrote the fromList function, I did not define insert inside it as an inner function. The insert function is useful on its own, quite apart from the functionality of fromList. So I wrote insert as a separate function. Even though fromList is the only function in my code that actually uses insert, that might not necessarily be true in the future. I might write a fromArray function, where I don't want to reuse fromList for efficiency's sake. (I could write fromArray as let fromArray a = a |> List.ofArray |> fromList, but that creates an unnecessary list that I'm just going to throw away when I'm done; it makes more sense, efficiency-wise, to directly iterate over the array and call insert as appropriate.)

So there's an example of when it's wise to use nested inner functions vs. separate functions in the same module. Now let's look at your code.

  • yDirection - This is a variable, but could be turned into a function taking piece as a parameter. As a function, it looks like it could be useful in many different functions. My judgment: separate.
  • sourceX and sourceY - These are variables, not functions, but you could turn that match into a function called source that returns a tuple, and then call it in your optionsFor function to set the values of sourceX and sourceY. In my opinion, that source function makes most sense as a separate function.
  • optionsForPiece - This function looks tightly associated with the optionsFor function, such that you probably wouldn't want to call it from elsewhere. My judgment: nested.
  • availableSelection - This could be quite useful in several situations, not just optionsFor. My judgment: separate.
  • availablePositions - This is a variable, but could easily be turned into a function that takes positions as a parameter and returns which ones are available. Again, that could be useful in several situations. My judgment: separate.

So by splitting out all the functions that seem like they could be re-used, we've gotten your optionsFor function down to the following:

// Functions yDirection, source, availableSelection,
// and availablePositions are all defined "outside"

let optionsFor piece (positions:Space list) =

    let yDir = yDirection piece

    let sourceX , sourceY = source piece

    let optionsForPiece pos = 
        pos = ((sourceX - 1) , (sourceY + yDir)) ||
        pos = ((sourceX + 1) , (sourceY + yDir))

    positions |> availablePositions |> List.filter optionsForPiece

That's a lot more readable when you revisit the code later, plus you get the benefit of having more reusable functions (like availableSelections) around for when you write the next bit of your code.

Upvotes: 4

Related Questions