Good practices with nested methods

[Mike Torrettinni 198]

I often use nested methods, mostly when a method needs 'helper' methods that are only applicable to it's logic.

Here is how I usually setup my nested methods:

 

procedure MainMethod(aParam: integer);

// definitions can be accessed also from nested methods
type
  TLocalUsed1 = record
    Field1: integer;
  end;

var vArray: TArray<TLocalUsed1>;

  // nested methods
  procedure LocalMethod_WorkOnArray;
  var i: integer;
  begin
    for i := Low(vArray) to High(vArray) do
      // work on array
  end;

  function GetDataFromArray: integer;
  var i: integer;
      ab: TLocalUsed1;
  begin
    for i := Low(vArray) to High(vArray) do
      // work on array
  end;

// definitions only accessible from main method
type
  TLocalUsed2 = record
    Field1: integer;
  end;

var i: integer;
   ab: TLocalUsed2;
begin
  // use local vars and nested methods
  //...
end;

 

I used to have var i defined on top of all nested methods and I had issues with using i in main and nested methods, until I figured out the difference in scope of definitions.

 

This works for me, but I was just thinking if there are any tips and tricks that can be used with nested methods, or some unique ways that you figure out how to use nested methods.

 

Any feedback on using (or not using) nested methods is appreciated!

 

Thanks!

 

 

 

 

 

 

[Rollo62 538]

I would rename one of the

var i: integer;

to to a distinct variable name "J", to avoid naming issues in the future.
Better safe than sorry.

 

Usualy when I use nested functions, they might also work on the above defined variables in the parent method,
so that these methods might act as a kind of "helper methods".

The drawback is that the common variables act like global variables, so you have to take care about their side-effects.
I use this usually only with very short methods, where the behaviour is clear (like Item_Add(const AValue : String);  ),
to preset repeated settings in one more convenient common method.

 

[Der schöne Günther 316]

Nothing against nested methods (altough I never use them), but what you show is way over the top. Let's put all these things in a single class.

[Arnaud Bouchez 407]

A quick and simple rule would be to define only mandatory variables as shared for the nested procedures, i.e. above them.

That is, try to keep all variables of the main block in the lower part of the code, just before the main block.
Just as you did.

 

From my experiment, nested functions are fine if there is only 1 (or 2) of them.
Once I reach the number of 2/3 nested functions, I usually define a private record type, with proper methods. It is kept private in the implementation section of the unit.
The trick of using a record with methods, and not a class, allows to allocate it on the stack in the main method, with no try..finally Free block.

It is much cleaner (more SOLID), and also much more extendable.

The main benefit is that it is re-usable: you could reuse some code, in another function.
Last but not least, you have the possibility to add a unitary test method, and browse easier the code in your IDE.

Edited September 5, 2019 by Arnaud Bouchez

[Bill Meyer 337]

A point not yet mentioned is the use of nested methods in the course of refactoring. In legacy code, it is not uncommon to find huge routines which have been thoughtlessly coded, and often with useless naming. I have found it helpful to remove to nested methods the chunks of code which might better be placed in a private class, as Arnaud suggests. Making them methods of the class in which you are working often makes no sense because these routines may have no practical value out of the context in which they were found. In new design, their use is harder to justify, I think.

 

[Arnaud Bouchez 407]

Yes, @Bill Meyer is right: do not put the nested functions part of the main class. It would break SOLID principles for sure - at least the single responsibility. Always favor composition in OOP.

 

About refactoring and breaking huge functions, you could take a look at those slides:

 

Following these proposals, for code refactoring, I would move the code into dedicated records with methods (or classes).

[David Heffernan 2347]

Nested methods are one of the biggest things stopping me porting my code to C++.

[Mike Torrettinni 198]

9 hours ago, Rollo62 said:

I would rename one of the

var i: integer;

to to a distinct variable name "J", to avoid naming issues in the future.
Better safe than sorry.

 

Usualy when I use nested functions, they might also work on the above defined variables in the parent method,
so that these methods might act as a kind of "helper methods".

The drawback is that the common variables act like global variables, so you have to take care about their side-effects.
I use this usually only with very short methods, where the behaviour is clear (like Item_Add(const AValue : String);  ),
to preset repeated settings in one more convenient common method.

 

Yes, I was using this approach, but it got messy when I needed J var in both scopes, so as long as I define all iteration vars after all nested methods, all is good.

 

 

 

[Clément 148]

Hi,

 The code you posted is full of traps! While it's ok to play with scope you could end it with unexpected behavior when debugging  should someone alter the disposition of the declarations ( There's always this guy that wants to optimize stuff). Another thing I always keep in mind. Nested methods are good candidate to private methods. Be sure to write safe code when future comes and avoid breaking working code because you WILL cut and paste that code and WILL suffer the cut and paste syndrome !

[Rollo62 538]

53 minutes ago, Clément said:

Nested methods are good candidate to private methods. 

Good point. I didn't count the many times when extracting from fast demo code with nested methods to private methods.
Maybe this is the kind of "Evolution" how code might evolve over time, in the process of testing different approaches for a solution.

[Arnaud Bouchez 407]

I don't agree - don't add new private methods. It is not SOLID. Try to keep the class definition small and readable.

Rather add a hidden object or class in your implementation section.

[Mike Torrettinni 198]

2 hours ago, Arnaud Bouchez said:

I don't agree - don't add new private methods. It is not SOLID. Try to keep the class definition small and readable.

Rather add a hidden object or class in your implementation section.

I think I prefer implementation section vs class private section because of Type definitions... they fit into implementation section much neater than in class private section - it's the visual/alignment thing.

[David Heffernan 2347]

But they have a much wider scope. Not a problem if your units have only one class. But with many classes it can be an issue. 

[Mike Torrettinni 198]

Aha, yes, I see what you mean. Yes, the scope is whole unit then. It's annoying when I create new class and I need to set Types above the class - these definitions immediately become global types - adding complexity to the code completion and error insight mess.

[Fr0sT.Brutal 900]

Nested methods are fine. Just place as many variables as possible after them to avoid side effects.