Class fields as parameters or refer to fields directly?

[Mike Torrettinni 198]

This question keep getting stuck in my head and I wonder which way should I do it:

Does anybody have a good rule or practical advice when to use class fields directly and when to pass them as parameters, when used in class methods?

 

For example - simple public Search method:

// public method
procedure TMyClass.Search(const aFieldName: string);
begin
  fFieldName := aFieldName; // fFieldName is TMyClass field
  
  // TMyClass.SearchByFieldName
  SearchByFieldName;
  // or TMyClass.SearchByFieldName(const aFieldName: string)
  SearchByFieldName(fFieldName); // or SearchByFieldName(aFieldName);
end;


procedure TMyClass.SearchByFieldName;
begin
  DoTheSearch1(fFieldName);
  DoTheSearch2(fFieldName);
end;

// or

procedure TMyClass.SearchByFieldName(const aFieldName: string);
begin
  DoTheSearch1(aFieldName);
  DoTheSearch2(aFieldName);
end;

I keep trying to justify one of the other approach, and a lot of times I justify passing parameters in case I need to extract method, and use it as standalone or copy to another class. In most cases this doesn't happen.

I guess this is left from 'old' style where all methods were organized by units, not classes. There you need parameters.

 

Any advice is welcome, thanks!

 

[Remy Lebeau 1409]

The only time this should make any real difference is if Search() is re-entrant.  If it is not, then don't worry about this, use whatever style you are comfortable with.  But if it is, then you can't use class members, as they could get overwritten during one Search() while another Search() is still running.

[Mike Torrettinni 198]

9 minutes ago, Remy Lebeau said:

as they could get overwritten during one Search() while another Search() is still running.

That's a good point! I assume a recursion is probably meant here. Not using many of them, but I will be careful.

[Guest]

One more point :

If you thinking of keeping TMyClass extensible, lets say here you might override or overload that Search, in the case of override your class will have different search method then using parameters will be logical use, on other hand if you will override it then using fields is the choice that will make sense, because with override you might use a field or not or different fields, override with no parameters will also make more sense and will be cleaner code if you are writing class abstraction. 

[Fr0sT.Brutal 900]

There's a strong opinion pro using parameters for the sake of testability. Some people even insist on using pure functions (those which do not use external variables at all) as much as possible. But everything depends on your needs and use cases. My opinion is to try with parameters; if you need to tie some values to an object, then use fields

[Stefan Glienke 2009]

My first impulse was pure function! as well but that depends on how SearchByFieldName is implemented - ultimately this looks like it mutates internal state.

However you should avoid temporal coupling as it leads to hard to maintain code.

[Mike Torrettinni 198]

Eh, last night I decided to go with 'use fields by default, unless there is a reason for parameters'. I was happy with this, I had a plan. Now, I see different suggestions. 🙂 

3 hours ago, Fr0sT.Brutal said:

Some people even insist on using pure functions (those which do not use external variables at all) as much as possible.

I only have this implemented in a class that has 100+ methods and I was testing if here is any difference between method using aClassMainData as parameter of fClassMainData field.

But I kind of like class methods, for the 'perceived' tidier implementation.

 

 

4 hours ago, Kas Ob. said:

lets say here you might override or overload that Search,

Good point. In this case I would definitely rethink what needs to be used. 

 

 

1 hour ago, Stefan Glienke said:

My first impulse was pure function!

 

1 hour ago, Stefan Glienke said:

However you should avoid temporal coupling as it leads to hard to maintain code.

Well, this is getting a little too much, I'm not ready for Interfaces, yet. Classes first.

But, isn't solution for temporal coupling just simple checking if class was initialized, if needed data is present?

b := TEndpointAddressBuilder.Create;
e := b.ToEndpointAddress();

function TEndpointAddressBuilder.ToEndpointAddress: string;
begin
  Result := '';
  if fURI <> '' then
    Result := ConvertURIToEndpointAddress;
end;

in this case assigning e will not fail in runtime.

Right, or am I missing the point of temporal coupling?

[Stefan Glienke 2009]

44 minutes ago, Mike Torrettinni said:

I only have this implemented in a class that has 100+ methods

Oh gosh...

 

As for what temporal coupling means - use google before I spend my time explaining it to you - someone else probably already did a better job anyway.

 

P.S. Your use of "class field" and "class method" is wrong - what you are referring to are instance fields and methods.

 

type
  TFoo = class
    fName: string; // instance field or just field - because every instance has its own
    class var fCount: Integer; // class field because there is just one - basically like a global variable but only visible inside of TFoo
    procedure Bar; // instance method or just method - can access instance members (like fName) but also class members (like fCount) - Self refers to the instance of TFoo
    class procedure SomeStuff; // class method - can access class members (like fCount) but not instance members - Self refers to the class itself - here TFoo - many other languages don't have something like this
    class procedure MoreStuff; static; // static class method - can also access class members but has no Self
  end;

 

Edited August 12, 2020 by Stefan Glienke

[Mike Torrettinni 198]

Ok, no worries, will try to avoid temporary coupling, as suggested.

[Mike Torrettinni 198]

2 hours ago, Stefan Glienke said:

P.S. Your use of "class field" and "class method" is wrong - what you are referring to are instance fields and methods. 

 

type
  TFoo = class
    fName: string; // instance field or just field - because every instance has its own
    class var fCount: Integer; // class field because there is just one - basically like a global variable but only visible inside of TFoo
    procedure Bar; // instance method or just method - can access instance members (like fName) but also class members (like fCount) - Self refers to the instance of TFoo
    class procedure SomeStuff; // class method - can access class members (like fCount) but not instance members - Self refers to the class itself - here TFoo - many other languages don't have something like this
    class procedure MoreStuff; static; // static class method - can also access class members but has no Self
  end;

 

Thanks, I didn't realize it.

[Remy Lebeau 1409]

13 hours ago, Mike Torrettinni said:

That's a good point! I assume a recursion is probably meant here. Not using many of them, but I will be careful.

Recursion, or multi-threading.

[Mike Torrettinni 198]

48 minutes ago, Remy Lebeau said:

or multi-threading.

Thanks, I don't use it very much, yet. Good to know for the future.

[HeZa 3]

On 8/12/2020 at 3:26 AM, Mike Torrettinni said:

Does anybody have a good rule or practical advice when to use class fields directly and when to pass them as parameters, when used in class methods?

If you call SearchByFieldname always with fFieldName as parameter then don't use a parameter because it is totally normal for a method to use fields of it own class.

 

[Mike Torrettinni 198]

7 minutes ago, HeZa said:

If you call SearchByFieldname always with fFieldName as parameter then don't use a parameter because it is totally normal for a method to use fields of it own class.

 

Thank you, good approach.

[Lars Fosdal 1792]

I prefer using method parameters instead of fields as parameters for reentrancy and thread safety.

 

This allows for a parallel model or recursive model

Result := SharedObject.Search('Value');

vs this, which does not allow parallel calls or recursive calls

SharedObject.SearchField := 'Value';
Result := SharedObject.Search;

 

[David Schwartz 427]

I prefer using parameters just to make it clear what variable is being passed. I've had situations where I moved methods around and the implied variables change and suddenly things break.

 

When I pass in a variable to a method as a parameter, it's a lot easier to see that I need to change it if I move something than if I let the method use an implied variable.

 

A lot of times when debugging, it's easy to miss WHICH variable of the same name is being used.

 

An example where this happens for me frequently is when I set up a nested method inside of an existing method, and then for some reason I decide to move it outside of that method (no longer nested) and the variable is also declared as a class variable, so there's no compiler error, but at run-time it suddenly does weird things because the value is coming from another scope now. (When I make nested methods, I want them to be really short and sweet, even if that increases coupling. It often comes back to bite me in the ass later on.)

Edited August 18, 2020 by David Schwartz

[Mike Torrettinni 198]

1 hour ago, David Schwartz said:

I prefer using parameters just to make it clear what variable is being passed. I've had situations where I moved methods around and the implied variables change and suddenly things break. 

Another good reason for parameters.

I'm looking into VirtualTrees.pas (Virtual treeview) to see how it is implemented there, and I'm starting to understand. Still a long way to not question what to do every time, but with experience I will.

 

Like everybody here is saying: there's no one exact rule, just guidelines.