Jump to content
Mike Torrettinni

Good practices with nested methods

Recommended Posts

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!

 

 

 

 

 

 

Share this post


Link to post

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.

 

Share this post


Link to post

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 by Arnaud Bouchez
  • Thanks 1

Share this post


Link to post

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.

 

  • Thanks 1

Share this post


Link to post

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).

  • Thanks 1

Share this post


Link to post
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.

 

 

 

Share this post


Link to post

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 !

Share this post


Link to post
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.

Share this post


Link to post

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.

  • Like 1

Share this post


Link to post
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.

Share this post


Link to post

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.

Share this post


Link to post

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

×