Seeking guidance for a constructor calling a constructor

[Warfley]

To give my opinion to the matter at hand, it depends on the context, the second option has an additional function, which introduces additional complexity. If you don't need it (as already stated, e.g. as a reset function), I don't se a reason why adding this additional complexity would help.

In general I find it really helpful to reduce all constructors to a single constructor. So with your example, I would go further:

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

1. // Base constructor
2. constructor TSomeClass.Create(const AStringField: String; AIntField: Integer);
3. begin
4.   inherited Create; // I will rant about this a little later :)
5.   FIntField:= AIntField;
6.   FStringField:= AStringField;
7. end;
8.  
9. // specialized empty constructor
10. constructor TSomeClass.Create;
11. begin
12.   Create(0, '');
13. end;
14.  
15. // specialized json constructor
16. constructor TSomeClass.Create(const AJsonString: String);
17. begin
18.   // parse json
19.   Create(parsedIntField, parsedStringField); // call with values from JSON
20. end;

The reason for this is simple, if you introduce a new field that needs to be provided at the point of construction, you just need to add it to the base constructors parameter list, and then you cannot forgett any other constructor, because if you forgett adding that field somewhere else, the compiler will throw an error because the signature of the constructor call does not match anymore. So this helps to minimize errors, because those errors will happen (forgetting to initialize a field in a constructor is one of the kind of bugs I encounter regularly) and when they happen you want the compiler to tell you.

This brings me right to another thing that many pascal programmers seem to forget from time to time, that is, that constructors have names. You don't need to name all constructors Create, you can actually give meaningful names. It's one of the most amazing features of Pascal in contrast to for example C++ or Java. So what I would do is the following:

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

1. // Base constructor
2. constructor TSomeClass.Create(const AStringField: String; AIntField: Integer);
3. begin
4.   inherited Create; // I will rant about this a little later :)
5.   FIntField:= AIntField;
6.   FStringField:= AStringField;
7. end;
8.  
9. // specialized empty constructor
10. constructor TSomeClass.Create;
11. begin
12.   Create(0, '');
13. end;
14.  
15. // specialized json constructor
16. constructor TSomeClass.CreateFromJson(const AJsonString: String);
17. begin
18.   // parse json
19.   Create(parsedIntField, parsedStringField); // call with values from JSON
20. end;

This allows for some neat things, for example, if you want to introduce a constructor that only takes the StringField and uses a base value for the IntField, you can simply now do that, and it does not collide with the JSON constructor (which also only takes a string) because they have a different name.
Then what you can also make the Base constructor private or protected, this allows it to only be called from other constructors and never be used to create the object directly. Therefore only the specialized constructors are available. This can readuce your code complexity, as you now have two options to put consistency checks, either in the base constructor, meaning if you have multiple constructors, you only need to put checks on the input values into the base constructor, meaning your specialized constructors can be dumb, or if your specialized constructors only allow very limited values in the first place (e.g. here the empty constructor will always call with 0 and ''), then consitency of inputs is already guaranteed through the specialized constructors, meaning you might not need any additional checks at all.

Then, above I added the "inherited" call to the base constructor. The reason for this is simple. There are cases in which you do not need  to call inherited, but doing it anyway does no harm (it pretty much just does nothing). But in those cases that you do need it, forgetting it will cause serious bugs (that are often hard to track). So doing it costs nothing and forgetting it can be really painful, therefore you should always use it. Thats one thing the compiler could help, with a warning if you forgett calling inherited on create, because this is something that should be in every class (and it is very easy to forgett, I myself often forgett it, even though I try to do it everywhere).

With the base constructor, this also makes tracking it really easy, because every class needs exactly one inherited Create call, and every constructor must lead to it (through other constructors). If I ever get around writing a Pascal linter (something I have on my list for quite some while now), I would add this as a rule to check, because checking it programatically is really easy, and an error if this is missing can save hours of debugging time because you forgott an inherited call somewhere.

So with this, some general things about the answers already given here:

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

When talking about the constructor, this code is always associated with a memory allocation. Allocating memory is orders of magnitude slower than any function call. So when running in performance problems with this, you shouldn't use classes for what you are trying to do anyway, but rather use something operating on pre-allocated memory, such as records or objects.
And generally speaking, this is what inlining is for. Simple functions should not add any overhead (except if they are virtual). That said, anyway, you should always try to develop for the most readable code in the first place, and only do such optimisations when you run into performance issues, and most importantly, when you have objective measures (like profiling) that this is a major source of delay. Most performance issues are not related to some microoptimisations like this, but rather algorithmic inefficiencies, or heavy hitting operations (such as memory allocations).
If you try to microoptimise your code even if you don't know if you will ever run into performance issues, you will simply end up with worse code for no benefit.

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 would be careful with inheritance, it is quite a useful tool, but more inheritance can result in worse code. There is this rather well known study: https://link.springer.com/article/10.1007/BF00368701 which assessed the maintainability of programs using building blocks with different levels of inheritance, and found that while small levels of inheritance (3 levels from base class to child class) can improve maintainability, but higher levels of inheritance (5+) do massively reduce maintainability. So at some point writing dublicate code is acutally better than using inheritance to outsource this.
Generally I think inheritance works best when it reflects some real concepts (for example a shape->Rectangle/Cicle), and is not just a tool to reduce some dublicate code in some methods. But of course it always depends on the actually data and architecture what makes sense and what doesnt.

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

This is actually somewhat dangerous, first because it requires a bottom value (i.e. a value without a meaning) for the parameter type. In this case it is the empty string, which seems fine at first, but massively limits your future possibilities. What if later on you decide you actually want to handle the empty string filename? Then you run into the same problem the socket api currently has, where they thought that as packets cannot have a size of 0, therefore reading 0 means end of stream, but later when non blocking sockets came around, suddenly you could have reads of 0 bytes, which now requires the awkward workaround of returning an error that basically means: "No error, just no data".

Second default parameters are generally dangerous in functions that might be overloaded or extended in the future. If you add another parameter of the same type afterwards also with default value, and then add this parameter to all calls of that function, the parameter could "slide" into the position of the former parameter, which was omitted previously, and the compiler will take it.
On a project I worked on, I found a bug like this that was dorment for over a year, I just found it by chaning one of the types, and then suddenly the function call was failing. We don't know how much of the data we had accumulated over the time (and used for evaluation) was corrupted through this bug, but we knew it was > 0.
Only use default parameters if you are quite sure the function signature won't change much more afterwards. And for constructors this is usually not the case

Lastly, it of course limits your overloading capabilities:

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

1. constructor Create; overload;
2. constructor Create(const AString: String); overload;
3. constructor Create(const AInteger: Integer); overload;
4. constructor Create(const AString: String; AInteger: Integer); overload;

Is something that is not possible with default parameters. So with all the stuff mentioned above, this is IMHO the preferrable way to write such a situation

[Warfley]

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.

Almost forgott this one

One thing I really like to use to design (data)classes around is the idea of immutable instances. This does not necessarily mean that the whole class is immutable (any working data of that class might still be mutable), but basically that classes which represent data from a source, are created on load, and then the data cannot change. This allows to greatly reduce the complexity, because when considering classes, you must always think about what references exist to that object. If you have a mutable object, which is referenced from two places, place A and B, and place A decides to change something about the class, then this must be known in place B, or place B will operate on the assumption that the value is still the same, even though it has changed.
This means you have to always know, at any point you use the class, where it can potentially be changed. And if the project is large enough, this isn't such an easy question to answer. Especially if you are new to the project, or return after a few years and doesn't know it inside out anymore, or you are working with others that might create or use such classes.

By making a class immutable, meaning all the data fields are only reachable via read only properties or getters, you already signalize on the source level, this instance will never change. This means you don't have to think about any of that. And as a neat side effect, immutable classes are pretty much by default thread safe, meaning creating thread safe programs is much easier with this.

The implementation of such is very easy, just load all the data in the constructor and never touch it again. But of course you might not always have an immutable datamodel, so for example in a customer database, the phone number of a customer might change. In this case you simply create a new object, with the same values in all the fields, except for the changed phone number. If it comes from a Database, you could update the database and then use the already existing "load from database" function to create a new instance with the changed data

This might sound very inefficient, thats because it is (as FPC does not have any optimisations for this, other languages like C# have an immutable class concept that allows the compiler to do optimizations which can make this even more efficient than regular classes), but as I said earlier, as long as performance is not an issue, there is no point in optimising for it. And for most applications on most modern hardware, performance is not an issue (in GUI applications a lag of 200ms is accaptable from a UX perspective, and you can create a lot of new objects in this amount of time). So if it creates cleaner code and reduces complexity, you should use it.

That said, because of manual memory management, this might result in worse code, because every time you mutate an object you need to free the old one. So I usually use this concept mostly in combination with either COM interfaces or records. It also matches nicely with operator overloading.
But that doesn't mean you shouldn't use this with classes at all, you just don't need to go all the way. For example you could add methods for changins small things, while the overall object stays intact. But, when you have a function that clears all the data of a class, that is usually an indication that at this point what you really want to do is to simply create a new object with the newly generated values, that you can pass to the constructor

[SymbolicFrank]

If you want to make sure all the classes in your class hierarchy are fully initialized, it is best to overload all the inherited constructors that don't have the needed parameters as private, and override the rest. A virtual method to Clear or Assign the fields, in the base class, seems to fix that problem, but not if you create instances in different descendants.

So, if constructors are only called from outside your classes: it works as intended. If they call constructors themselves: take care.

[mas steindorff]

<snip>

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 would be careful with inheritance, it is quite a useful tool, but more inheritance can result in worse code. There is this rather well known study: https://link.springer.com/article/10.1007/BF00368701 which assessed the maintainability of programs using building blocks with different levels of inheritance, and found that while small levels of inheritance (3 levels from base class to child class) can improve maintainability, but higher levels of inheritance (5+) do massively reduce maintainability. So at some point writing dublicate code is acutally better than using inheritance to outsource this.

Thanks Warfley,
I came up with a limit of 4 levels in my own testing several years ago but my 1st class tended to be empty and was there only for future options.  it good to see I was nearly normal 
I do appreciate your input.
MAS

[SymbolicFrank]

If you want all of your classes to share a set of traits, it is very useful to make an interface or abstract class that implements them and inherit all others from that or a descendant. That way, you can have wildly different classes that share those things. Case in point: TObject.