Seeking guidance for a constructor calling a constructor

[Gustavo 'Gus' Carreno]

Hey Y'all,

I've been quite contempt with the following code, and I even thought it was rather clever.
But, now, after letting it on the back burner for a while, I feel I'm missing something like caveats or pitfalls that I'm blind to.

Code: Pascal  [Select][+][-]

1. interface
2.  
3. type
4.   TSomeClass = class(TObject)
5.   private
6.     FIntField: Integer;
7.     FStringField: String;
8.  
9.     procedure LoadFromJSON(AJSON: String);
10.   public
11.     constructor Create;
12.     constructor Create(AJSONS: String);
13.   end;
14.  
15. implementation
16.  
17. procedure TSomeClass.LoadFromJSON(AJSON: String);
18. begin
19.   //  Load JSON into fpjson structures
20.   { ... }
21.   //  From fpjson structures set the fields
22.   { ... }
23. end;
24.  
25. constructor TSomeClass.Create;
26. begin
27.   FIntField:= 0;
28.   FStringField:= '';
29. end;
30.  
31. constructor TSomeClass.Create(AJSON: String);
32. begin
33.   Create;
34.   LoadFromJSON(AJSON);
35. end;

What I'm thinking is that I should refactor the code like thus:

Code: Pascal  [Select][+][-]

1. interface
2.  
3. type
4.   TSomeClass = class(TObject)
5.   private
6.     FIntField: Integer;
7.     FStringField: String;
8.  
9.     procedure InitiateFields;
10.     procedure LoadFromJSON(AJSON: String);
11.   public
12.     constructor Create;
13.     constructor Create(AJSON: String);
14.   end;
15.  
16. implementation
17.  
18. procedure TSomeClass.InitiateFields;
19. begin
20.   FIntField:= 0;
21.   FStringField:= '';
22. end;
23.  
24. procedure TSomeClass.LoadFromJSON(AJSON: String);
25. begin
26.   //  Load JSON into fpjson structures
27.   { ... }
28.   //  From fpjson structures set the fields
29.   { ... }
30. end;
31.  
32. constructor TSomeClass.Create;
33. begin
34.   InitiateFields;
35. end;
36.  
37. constructor TSomeClass.Create(AJSON: String);
38. begin
39.   InitiateFields;
40.   LoadFromJSON(AJSON);
41. end;

What does the community think about these two pieces of code?

I'm paging the experts, the ones that are more active: @PascalDragon, @marcov and @Martin_fr.

But I do, I really do, appreciate whatever nugget of wisdom anyone else can add to this in doubt mental state.

As per usual, you have my most sincere thanks in advance for your time and patience with my predicaments

Cheers,
Gus

[HeavyUser]

1) the more calls you add the slower you code becomes.
2) the prefix T is used only on type definition not on variables.
3) If you do use the initializeFields idea do make sure that at least its virtual.

Personally I used both methods, I used your second method in c++ where I had problems with virtual constructors and your first method as my primary go to method in pascal.

In short do not break your methods unless absolutely necessary and in your example breaking the initialization code out to a 3rd method is excessive.

The only way that might be acceptable if you had a reset method of some sort.

[mas steindorff]

looks like you have one approach figured out

another way, if your class is complex or may be useable in a simpler object...  is to define 2 classes, a base class "TSomebase" and then TsomeClass = class(TSomebase)

then in each of your class creates, do:
  inherited Create();
 ... and then your other init code

[bpranoto]

Hey Y'all,

I've been quite contempt with the following code, and I even thought it was rather clever.
But, now, after letting it on the back burner for a while, I feel I'm missing something like caveats or pitfalls that I'm blind to.

Code: Pascal  [Select][+][-]

1. ...
2. constructor TSomeClass.Create;
3. begin
4.   InitiateFields;
5. end;
6.  
7. constructor TSomeClass.Create(AJSON: String);
8. begin
9.   InitiateFields;
10.   LoadFromJSON(AJSON);
11. end;

In this case, I prefer to define only one constructor with Optional parameter like this:

Code: Pascal  [Select][+][-]

1. constructor TSomeClass.Create(AJSON: String='');
2. begin
3.   InitiateFields;
4.   if AJSON <> '' then  LoadFromJSON(AJSON);
5. end;

Simpler and clearer..

Regards,
Bambang

[Gustavo 'Gus' Carreno]

Hey HeavyUser,

1) the more calls you add the slower you code becomes.

Thanks for the tip. I'll keep this in mind when doing code that is called many times in a cycle.
This code is mainly for marshalling and un-marshalling from JSON <--> TObject.
Nothing that needs any speed tuning, I guess

2) the prefix T is used only on type definition not on variables.

  Shuck, mate, thanks!! Now I feel embarrassed for the stupid mistake. Let's put it down to being too early in the morning when I typed the code, LOL!!!
I've corrected the code to display the appropriate F for field.

3) If you do use the initializeFields idea do make sure that at least its virtual.

I think I've never fully understood what virtual means and how best to use it. Could you elaborate or point me out for a good quick refresher?
Many thanks in advance!!

Personally I used both methods, I used your second method in c++ where I had problems with virtual constructors and your first method as my primary go to method in pascal.

Good to know, many thanks!!

In short do not break your methods unless absolutely necessary and in your example breaking the initialization code out to a 3rd method is excessive.

The only way that might be acceptable if you had a reset method of some sort.

I agree that, in this very simplistic example, the InitiateFields method is a bit of an overkill.

But in my full code there are methods to LoadFrom[JSON|JSONData|JSONObject|JSONArray], which in my opinion, needs the InitiateFields as a reset button.

I will take you tips onboard as they are quite useful.
Many thanks for them!!

Cheers,
Gus

[Gustavo 'Gus' Carreno]

Hey mas steindorff,

looks like you have one approach figured out

Thanks

another way, if your class is complex or may be useable in a simpler object...  is to define 2 classes, a base class "TSomebase" and then TsomeClass = class(TSomebase)

then in each of your class creates, do:
  inherited Create();
 ... and then your other init code

I understand what your proposing, but having a parent class with just a single constructor to initiate the fields is a bit much in my view.
But yes, it would eliminate the fact that we're calling a constructor at the same level and not at parent level.

Cheers,
Gus

[Gustavo 'Gus' Carreno]

Hey Bambang,

In this case, I prefer to define only one constructor with Optional parameter like this:

Code: Pascal  [Select][+][-]

1. constructor TSomeClass.Create(AJSON: String='');
2. begin
3.   InitiateFields;
4.   if AJSON <> '' then  LoadFromJSON(AJSON);
5. end;

Simpler and clearer..

Bloody hell, this IS cleaner and simpler
Why didn't I think of that!?!?!?

Bambang, your a godsend!! Many thanks!!

Cheers,
Gus

[jamie]

Hmm

Code: Pascal  [Select][+][-]

1. Constructor TSomeClass.Create(N:String);
2. var
3.   T:TSomeClass;
4.  Begin
5.    Create;
6.    T := TSomeClass.Create(N);
7.  end;                          
8.  

Experiment with that one.

The Create constructor here seems like it is being called as a procedure basically so only one object is created.

[HeavyUser]

I think I've never fully understood what virtual means and how best to use it. Could you elaborate or point me out for a good quick refresher?
Many thanks in advance!!

Think of it like this, a virtual method allows you to override it in any child class, to extend it with extra fields that need initialization, if the method is not virtual, you will face problems on which method is called based on the type of the instance used, type casting in your code etc.

If on the other hand there are no child classes in the hierarchy the virtual becomes a speed bottleneck instead.

In short use virtual only when appropriate ee when there are child classes in the hierarchy that need to extend it.

That's my rule of thumb for virtual, for the technical details, vmt tables, jumps and all that, I'll leave it to people that have implement the virtual tables in the compiler, in fear that I might say something incompatible with the existing implementation.

I agree that, in this very simplistic example, the InitiateFields method is a bit of an overkill.

But in my full code there are methods to LoadFrom[JSON|JSONData|JSONObject|JSONArray], which in my opinion, needs the InitiateFields as a reset button.

I will take you tips onboard as they are quite useful.
Many thanks for them!!

makes sense to have it then.

[howardpc]

To add an example to HeavyUser's explanation.
"virtual" makes sense only in the context of a class hierarchy, in which you want an intelligently-named method propagated down descendants to have  customised effects for each member of the hierarchy. This avoids having to come up with synonyms of the base class's method name in each descendant in order to implement the same functionality differently at each level. Instead you declare the method virtual, and use the same method name throughout the hierarchy, but override it at each level to indicate that at the overridden level the method does something different. You have the option to invoke the base class's method functionality as well by calling inherited somewhere in the overridden method. In this case you have to consider whether the inherited call should come at the beginning or end of the overridden method (or whether it makes no difference when it is called).

A straightforward example would be descendants of TGraphicControl, which declares a virtual Paint method. It is obvious that TLabel.Paint and TSpeedButton.Paint need to do very different things, and so must be overridden in these two descendants. The virtual/override syntax means you can reuse the appropriate name "paint" without having to come up with ridiculous names like "daub", "sketch", "portray" etc. in your descendants' methods. If you redeclare, say, a Paint method in a descendent class when its ancestor Paint method is not declared virtual, you are prevented from calling inherited if needed, and this child Paint method is completely distinct from its ancestor's Paint method. In fact the duplicated name is then actually confusing, since it implies an ancestral  relationship that does not exist.

[Gustavo 'Gus' Carreno]

Hey HeavyUser and HowardPC,

Okies, it finally makes sense when you have a clear use case like you two state.

In my case it's not really needed due to one of your statements: The classes I'm implementing are somewhat final (Hope we could have this possibility), since they just encapsulate data coming from a JSON API. More akin to an Entity on an ORM. And due to this, will not participate on a hierarchy.

Nonetheless, you two have given me a good refresher on the virtual thing and for that I thank you profusely !!

Cheers,
Gus

[Gustavo 'Gus' Carreno]

Hey Jamie,

I very much admire your knowledge, and I've had good examples of it in the past, but sometimes you come up with an answer that completely baffles me...

Hmm

Code: Pascal  [Select][+][-]

1. Constructor TSomeClass.Create(N:String);
2. var
3.   T:TSomeClass;
4.  Begin
5.    Create;
6.    T := TSomeClass.Create(N);
7.  end;                          
8.  

Experiment with that one.

The Create constructor here seems like it is being called as a procedure basically so only one object is created.

This is one of those times

I kinda understand what you're getting at, and it was one of my fears(Having 2 objects instantiated when calling a constructor inside another constructor), but in your example, the scope of T is only inside that constructor and it does not come outside it, so I'm baffled...

Another thing that baffles me is: How on earth is the line below the Create gonna change the private fields on the object that is being created by the call to Create(N)?

Also, in this implementation I feel that we fall inside a never ending recursion, am I right?

Dear Jamie, did you think this one through?

Cheers,
Gus

[jamie]

Also, in this implementation I feel that we fall inside a never ending recursion, am I right?

Dear Jamie, did you think this one through?

 

[Zvoni]

Hey Gus,
i agree with bambang's approach.
Your second approach has something that is easily overlooked (though it's a very minor "issue")
You set Values for your Fields twice!
First with the Procedure-call to some default-values, and then you overwrite them with your JSON.
Another approach might be to still use your Initiate-Procedure but with Arguments

If constructor is called without JSON:
Initiate(0,'');
If with JSON:
Initiate(IntFromJSON, StringFromJSON);

My 2 €-cents

[PascalDragon]

I'm paging the experts, the ones that are more active: @PascalDragon, @marcov and @Martin_fr.

I would throw out the complete initialization, cause class instances are already implicitly initialized by the RTL, so FIntField already has the value 0 and FStringField already has the value ''. You only need to set something manually if you need a value that's not 0 or equivalent.