Overload methods or use unique names?

[Mike Torrettinni 198]

Do you often use Overloaded methods or you try to avoid them? 

I keep getting into this dilemma, especially because I want to use Overload instead of unique method names, but eventually overloaded methods become too complex.

For example:

Let's say we have TaskScheduler class and it needs methods to add tasks, and here how it started:

 

TTaskAction = (taNa, taExecuteNow, taExecuteDelayed, taQueueTaskOnly...);

procedure AddTask(aAction: TTaskAction; aTask: TTask); overload; // Execute aTask immeditaly

procedure AddTask(aAction: TTaskAction; aTask: TTask; aExecDateTime: TDateTime; aAllowImmediateExec: boolean); overload; // Execute aTask at set date time

procedure AddTask(aAction: TTaskAction; aTask: TTask; aQueueOnly: boolean); overload; // Put aTask to Queue and wait for execution

this looks good, but as soon as you start adding more info for each TTaskAction, it become a mess because some parameters get passed empty, depending on which TTaskAction is being used for.

 

This is easy solved if I use AddImmediateTask, AddDelayedTask and AddTaskToQueue methods. Then no mess.

 

Any recommendations how to approach this or should I just use overloaded methods for very simple cases?

 

[ConstantGardener 31]

...for this example you could work without the TTaskAction at all. For me the parameterlist determinate the action!

[FredS 138]

12 minutes ago, ConstantGardener said:

you could work without the TTaskAction

Yup, or for readability (10+ years later) add it as a last parameter with a default value.

[Der schöne Günther 306]

Refrain from methods with half a dozen parameters, that's just not readable anymore.

https://sourcemaking.com/refactoring/smells/long-parameter-list

 

Use a parameter object, and make the values nullable/optional. Then you don't even have overloads for your method anymore. And validation can happen in the scope of your argument object. Like this:

 

type
    TMethodArgument = record
        action:            TTaskAction;
        task:            TTask;
        schedule:        Nullable<TDateTime>;
        isQueueOnly:    Boolean;
        hasImmediateExecution: Boolean;
        
        class function Default(): TMethodArgument; static;
        function getIsValid(): Boolean;
    end;
    
procedure TMyClass.someMethod(const arg: TMethodArgument);
begin
    if (not arg.isValid()) then
        raise EArgumentException.Create(..);
        
    // your stuff here...
end;

 

[David Heffernan 2288]

If the methods perform the same task, use overloads. What you have here looks like it should definitely use overloads. 

 

[Der schöne Günther 306]

Also, maybe I'm a bit of an extremist, but I loathe default parameters. I think Delphi still has that bug where you can't serialize a TDictionary<X, Y> because RTTI looks up a parameterless constructor, but the constructor of the TDictionary actually is something like "Create(OwnsObjects: Boolean = False)" which is not the same and the constructor never gets called and you end up with an object in a zombie state.

 

Coding guides like MISRA/AUTOSAR (M8-3-1), HIC++ (Kap. 9.1.2) or even Google agree on default parameters.

Edited December 13, 2019 by Der schöne Günther

[Mike Torrettinni 198]

2 hours ago, ConstantGardener said:

...for this example you could work without the TTaskAction at all. For me the parameterlist determinate the action!

This is odd... the TTaskAction controls the behavior of each Task/how task is executed... for example: RunTasks method executes each task based on TTaskAction value.

You are saying that all logic related to task should check all values (parameters in AddTask) and then decide the execution?

 

Probably I don't understand what you mean, because this makes no sense to me, that would be a big mess of IFs.

 

 

[Lars Fosdal 1727]

I prefer individually named methods instead of overloads.

Optional parameters can work in some cases, but when things get really complex with a lot of variations, I use variations on the theme that @Der schöne Günther introduced. A record, an object, or an anonymous method.

 

 

 

 

[Guest]

4 hours ago, Mike Torrettinni said:

This is easy solved if I use AddImmediateTask, AddDelayedTask and AddTaskToQueue methods. Then no mess.

I prefer AddTaskImmediate, AddTaskDelayed and AddTaskToQueue, without the English word order correctness, i find it easier to find, read, and browse the code, as the declaration will be in one place (the eye read them faster) and those methods will be neighbors in implementation, the most important will be prettier in code complete.

 

I always use the overloads for the simple jobs, like a method called TBytesFrom where the parameters is ( string, ansistring, stream, record ....) 

[Mike Torrettinni 198]

3 hours ago, Kas Ob. said:

I prefer AddTaskImmediate, AddTaskDelayed and AddTaskToQueue, without the English word order correctness, i find it easier to find, read, and browse the code, as the declaration will be in one place (the eye read them faster) and those methods will be neighbors in implementation, the most important will be prettier in code complete.

 

I always use the overloads for the simple jobs, like a method called TBytesFrom where the parameters is ( string, ansistring, stream, record ....) 

I think my examples were not clear enough how complicated it gets when you add additional info, like TaskName, TaskPath, RunLevel, QueueID.... parameters that define each TTaskAction. So, for this case, I renamed all methods.

 

[Mike Torrettinni 198]

5 hours ago, Der schöne Günther said:

Refrain from methods with half a dozen parameters, that's just not readable anymore.

https://sourcemaking.com/refactoring/smells/long-parameter-list

 

Use a parameter object, and make the values nullable/optional. Then you don't even have overloads for your method anymore. And validation can happen in the scope of your argument object. Like this:

 

type
    TMethodArgument = record
        action:            TTaskAction;
        task:            TTask;
        schedule:        Nullable<TDateTime>;
        isQueueOnly:    Boolean;
        hasImmediateExecution: Boolean;
        
        class function Default(): TMethodArgument; static;
        function getIsValid(): Boolean;
    end;
    
procedure TMyClass.someMethod(const arg: TMethodArgument);
begin
    if (not arg.isValid()) then
        raise EArgumentException.Create(..);
        
    // your stuff here...
end;

 

This is interesting approach, it could work on some methods where I have 10+ parameters and then half of the calls pas string and integer values, while the pother half passes '' or 0.

 

This just means I need to create/prepare record before the call. Which is kind of moving the logic from the called method into the callers... right?

[David Heffernan 2288]

10 hours ago, Mike Torrettinni said:

Which is kind of moving the logic from the called method into the callers... right?

No. The caller has to tell the callee what to do. The callee needs to get the information from somewhere.

 

I wonder, do you have any good books on software design? I really don't think this sort of question has proved fruitful. 

[Mike Torrettinni 198]

 

5 hours ago, David Heffernan said:

No. The caller has to tell the callee what to do. The callee needs to get the information from somewhere.

 

I wonder, do you have any good books on software design? I really don't think this sort of question has proved fruitful. 

Aha, yes, correct! In one of my cases I have to prep some data from other arrays to define record to pass as parameter. Which is not really a logic, so I was thinking of this.

 

It probably doesn't seem useful to you as it is to a developer of my level... every feedback is usually a lot more valuable than direct answer to a problem. I can see how other developers do it, suggestions how I might do it in the future, or sometimes I just combine from different developers.

[David Heffernan 2288]

It's not an effective way to learn. A really good book is what you need. In my opinion. 

[Mike Torrettinni 198]

@David Heffernan In your experience, even if the methods do perform same tasks, is there a limit when you don't overload them? Like too many different parameters, like in my case? Or maybe you decide on other criteria...

[David Heffernan 2288]

I don't think an answer to that question will help you. 

[Attila Kovacs 617]

Strange, if you look at your original example, the proper function names are already there, at the end of each line, in the comments.

[Mike Torrettinni 198]

Just now, David Heffernan said:

I don't think an answer to that question will help you. 

OK, I assume you don't use overloaded methods. Which is good answer for me.

[Mike Torrettinni 198]

7 minutes ago, Attila Kovacs said:

Strange, if you look at your original example, the proper function names are already there, at the end of each line, in the comments.

Yes, I was trying to decide if I should use overloaded methods no matter how complicated parameters are, or just use unique names.

[Attila Kovacs 617]

procedure AddTask(aAction: TTaskAction; aTask: TTask); overload; // Execute aTask immeditaly

Does it make sense?

 

AddTask(taQueueTaskOnly, lTask); // Execute aTask immediately

 

 

Edited December 15, 2019 by Attila Kovacs

[Mike Torrettinni 198]

5 minutes ago, Attila Kovacs said:

procedure AddTask(aAction: TTaskAction; aTask: TTask); overload; // Execute aTask immeditaly

Does it make sense?

 

AddTask(taQueueTaskOnly, lTask); // Execute aTask immediately

 

 

If with ITask you are suggestion similar as @Der schöne Günther , then the TTaskAction (taQueueTaskOnly) is part/property of ITask. Then it could be even simpler AddTask(ITask), right?

[David Heffernan 2288]

2 hours ago, Mike Torrettinni said:

OK, I assume you don't use overloaded methods. Which is good answer for me.

I do, sometimes, use overloaded methods.

 

You are simply not going to learn anything by asking people to tell you how the decisions to make when they don't have the necessary information to make them. You need to understand concepts but you just seem to ask us to tell you what to do in a specific case. 

[Mike Torrettinni 198]

 

11 minutes ago, David Heffernan said:

I do, sometimes, use overloaded methods.

I only have 1 really good usage of overloaded methods example in my projects, and that is IfThen, with a lot of different types of parameters. But these only come with 3 parameters, like:

 

function IfThen(const AValue: boolean; const ATrue: TVirtualStringTree; const AFalse: TVirtualStringTree = nil): TVirtualStringTree; overload; inline;

This works great!

 

And then I have example such as:

 

function LogMissingItem(
  const aProject: integer;
  const aProjectSection: TProjectSection;

  const aProjectType: TProjectType;

  const aItemIdx, aItemDataIdx, aItemXMLID, aItemSubID: integer;

  const aItemReferencedID, aReferencedXMLID: integer;

  const aCategoryID: integer = 0; const aSubCategoryID: integer = 0;

  const aProjParameters: boolean = false;
  const aProjID: integer = 0;
  const aItemProp: string = '';
  const aItemSubProp: integer = 0;
  const aProjSubID: integer = 0): boolean;

And LogMissingItem is called for 10+ cases of data, and a lot of parameters are 0s on empty strings, because not every parameter is applicable to every Project Item Log entry.

 

So, I can now create numerous overloaded LogMissingItem  methods, but I think in this cases I will actually implement @Der schöne Günther example, with record as parameter. Much better use case,

[Fr0sT.Brutal 897]

I prefer overloaded methods, but only for cases of

1) the same set of parameters differing only in types (good example is IfThen)

2) several parameter sets (2-3)

Dozens of methods significantly differing in param sets are usually hard to understand. Quite often I add an overload to utilize a default parameter of a type that Delphi doesn't allow to set in code, like record, object or array (like Error(Msg) and Error(MsgFmt, [values])).

Edited December 16, 2019 by Fr0sT.Brutal

[David Schwartz 412]

You have a TTask object and you are defining different ways of adding it to a TTaskScheduler object and when to start it running.

 

Initially, I thought the examples in the first post are global-scope methods, but there's no queue argument, so I'm going to guess they're members of the TTaskScheduler class.

 

So you have a Task Scheduler object where you want to add tasks with different initial start attributes.

 

The normal (default) case when adding a task to a scheduler is that it's blocked; you'd add the task, set some properties, then start it running. It's an initialization sequence.

 

var
  t : TTaskID;
  . . .  
begin
  . . .
  t := q.Add( task1 );
  task1.prop1 := xxx;
  task1.prop2 := yyy;
  q.Resume( t );
  

 

In a situation where the task is fully initialized or can run with default settings, then there's usually a way to add it and start it running in a single call.

 

  
  t := q.AddAndRun( task1 );

  // equivalent to this:

  q.Resume( q.Add( task1 ) );
 

 

Both Add cases need to return a handle of sorts that identifies the task in the queue, referred to as TTaskID here. It's an index related to the queue because that's what you're adding it to. (It could also be a TTaskSchduler reference or possibly something else.) Why? Because using a TTask reference forces you to search the queue to find the task in question. It takes O(n/2) time which will vary with the size of the queue, rather than O(1) which is constant.

 

The second option where you're delaying the start to a specified time is like adding a task normally that starts out blocked, but you set its starting time instead of calling Resume.

 

So it's really a variation on Resume, not Add.

 

This is what I'd do:

 

type 
  TTaskID = integer;
  
TTaskScheduler = class
  . . .
public
  function Add( aTask : TTask ) : TTaskID; // add task to the queue and leave it blocked
  function AddAndRun( aTask : TTask ) : TTaskID; // add a task to the queue and start it running immediately

  function Suspend( aTaskID : TTaskID ) : TTaskID;  // stops the task
  function Resume( aTaskID : TTaskID ) : TTaskID;  // starts running immediately
  function ResumeAt( aTaskID : TTaskID; aExecDateTime: TDateTime ) : TTaskID;  // resumes the task at the appointed time
  function ResumeAfter( aTaskID: TTaskID; aMsecDelay : Cardinal ) : TTaskID;  // resumes the task after some delay
  . . .
end;

 

So in a way, the question is irrelevant in this case when you get the design right (IMHO) and name things properly.

 

Sorry, but this doesn't really address your question ... and yes, I do run into issues from time to time with too many overloaded methods, but the problem usually resolves itself when I look more closely at the design of the class and what the actual use cases are.

 

Note that you could get fancier with this, allowing expressions using a fluent notation like so:

 

q.Add( task1 ).Resume();  // in place of AddAndRun
q.Add( task1 ).Resume().At( dawn_tomorrow );
q.Resume( t ).After( Refresh_delay );

 

 

Edited January 2, 2020 by David Schwartz