Best way to implement Clone() in my specific classes

Question

In my specific TPersistent classes I'd like to provide a Clone function, which returns an independent copy of the object.

Is it possible to make this work correctly with descendants, without implementing the Clone function in each and every descendant?

This is not about cloning any unknown fields or deep cloning (which could be done using RTTI). In my minimal example below, you can see where I would want to put the Clone function.

Since it uses Assign() to copy the data, it would work with any descendant. The problem is the constructor, see comments. How do I call the correct constructor of that descendant? If that's very hard to do, it's okay to assume that none of the descendants override the constructor without overriding Clone, too.

program Test;

uses System.SysUtils, System.Classes;

type
  TMyClassBase = class abstract(TPersistent)
  public
    constructor Create; virtual; abstract;
    function Clone: TMyClassBase; virtual; abstract;
  end;

  TMyClassBase<T> = class abstract(TMyClassBase)
  private
    FValue: T;
  public
    constructor Create; overload; override;
    function Clone: TMyClassBase; override;
    procedure Assign(Source: TPersistent); override;
    property Value: T read FValue write FValue;
  end;

  TMyClassInt = class(TMyClassBase<Integer>)
  public
    function ToString: string; override;
  end;

  TMyClassStr = class(TMyClassBase<string>)
  public
    function ToString: string; override;
  end;

constructor TMyClassBase<T>.Create;
begin
  Writeln('some necessary initialization');
end;

procedure TMyClassBase<T>.Assign(Source: TPersistent);
begin
  if Source is TMyClassBase<T> then FValue:= (Source as TMyClassBase<T>).FValue
  else inherited;
end;

function TMyClassBase<T>.Clone: TMyClassBase;
begin
  {the following works, but it calls TObject.Create!}
  Result:= ClassType.Create as TMyClassBase<T>;
  Result.Assign(Self);
end;

function TMyClassInt.ToString: string;
begin
  Result:= FValue.ToString;
end;

function TMyClassStr.ToString: string;
begin
  Result:= FValue;
end;

var
  ObjInt: TMyClassInt;
  ObjBase: TMyClassBase;
begin
  ObjInt:= TMyClassInt.Create;
  ObjInt.Value:= 42;
  ObjBase:= ObjInt.Clone;
  Writeln(ObjBase.ToString);
  Readln;
  ObjInt.Free;
  ObjBase.Free;
end.

The output is

some necessary initialization
42

So, the correct class came out, it works correctly in this minimal example, but unfortunately, my necessary initialization wasn't done (should appear twice).

I hope I could make it clear and you like my example code :) - I'd also appreciate any other comments or improvements. Is my Assign() implementation ok?

@Victoria, that wouldn't work, since the resulting object would be of type `TMyClassBase` and not `TMyClassInt`, so it won't work. — maf-soft, Mar 22 '18 at 21:11

David Heffernan · Accepted Answer · 2018-03-23T13:09:00.410

2

You don't need to make the non-generic base class constructor abstract. You can implement the clone there, because you have a virtual constructor.

Furthermore you don't need to make the Clone method virtual.

type
  TMyClassBase = class abstract(TPersistent)
  public
    constructor Create; virtual; abstract;
    function Clone: TMyClassBase; 
  end;

... 

type
  TMyClassBaseClass = class of TMyClassBase;

function TMyClassBase.Clone: TMyClassBase;
begin
  Result := TMyClassBaseClass(ClassType).Create;
  try
    Result.Assign(Self);
  except
    Result.DisposeOf;
    raise;
  end;
end;

Note that ClassType returns TClass. We cast it to TMyClassBaseClass to make sure that we call your base class virtual constructor.

I also don't see why you made TMyClassBase<T> abstract and derived specifications from it. You should be able to implement everything you need in the generic class.

edited Mar 23 '18 at 13:09

answered Mar 22 '18 at 21:48

David Heffernan

601,492
42
1,072
1,490

@sertac Thank you – David Heffernan Mar 23 '18 at 07:03
Great, that was what I was looking for - my understanding of the mechanics seems to be incomplete. I couldn't imagine that `TThis(TThat).Create` could return an object of TThat. Also the `class of` which you first forgot wasn't clear to me. However, maybe you want to edit your answer - you made it look like the solution is moving the Clone function to the base class, but that's not necessary. Better first explain the solution and move that as an additional recommendation into a postscript... – maf-soft Mar 23 '18 at 07:20
Since you can implement it in the base class you may as well. Why wouldn't you? – David Heffernan Mar 23 '18 at 07:33
You are right and I will do so. I just think the first sentence shouldn't have the least important part of the solution. The solution I missed is the class-cast, that's the important part. Where I could put it better is additional info. Btw., is my `Assign` method good like it is? – maf-soft Mar 23 '18 at 07:36
You wrote "implement everything you need in the generic class": how would I do that? if..else for every possible type? – maf-soft Mar 23 '18 at 07:54
Put the value into a `TValue` and call its ToString method. Any time you find yourself implementing a method in a specialisation it's a bad sign. Always try to avoid that. – David Heffernan Mar 23 '18 at 08:08
`ToString` was just a simplified example. In real life my classes have to support exactly some few fixed types and cannot support all possible types. Of course you can always try to avoid it, but it's not automatically bad design. Let's say the output string has to be in quotes, the integer not. And even if it was exactly my example, wouldn't have the specialized `TMyClassStr.ToString` much better performance than feeding it through `TValue`? – maf-soft Mar 23 '18 at 09:03
Yes, but you also could not use `TMyClassBase`, you'd have to use `TMyClassInt`, so you wouldn't be able to use these classes in a generic setting. If you want to specialise based on the type, within a generic method, use `GetTypeKind` and the like. This is all resolved at compile time too so you don't suffer any performance hits. Anyway, we are now drifting far from the question you asked, which I think has been answered. – David Heffernan Mar 23 '18 at 09:15
2

@David you're welcome. A downvote made me realize the mistake and after a while I figured you'd call the day off.. – Sertac Akyuz Mar 23 '18 at 13:23
The great thing is that we got a general solution and could put it into a reusable `TCloneable` class. Then every descendant from this class automatically becomes cloneable (if it implements the `Assign` method). Note: when descendants need to introduce additional constructors, the developer has to take care that the parameterless `Create` + `Assign` still create a valid object. This pitfall might be the reason why there is no ready made `Clone`? – maf-soft Mar 23 '18 at 14:26

maf-soft · Answer 2 · 2018-03-24T07:30:15.150

-2

This seems to do it:

function TMyClassBase<T>.Clone: TMyClassBase;
begin
  {create new instance using empty TObject constructor}
  Result:= ClassType.Create as TMyClassBase<T>;
  {execute correct virtual constructor of descendant on instance}
  Result.Create;
  {copy data to instance}
  Result.Assign(Self);
end;

However, I've never seen that before - it feels very, very wrong...

I verified that it correctly initializes data of the target object and really calls the descendants constructor once. I see no problem, there is also no memory leak reported. Tested using Delphi 10.2.2. Please comment :)

edited Mar 24 '18 at 07:30

answered Mar 22 '18 at 17:38

maf-soft

2,335
3
26
49

1

You seem to call the constructor twice – David Heffernan Mar 22 '18 at 19:06
@DavidHeffernan, that would result in a memory leak, isn't it? But it doesn't. Actions of the second .Create go to the existing object. I can imagine good reasons to downvote this, but this reason is just not true. You can easily copy my whole example and try it out. How many "some necessary initialization" do you see? – maf-soft Mar 22 '18 at 21:16
Have your constructor create an object owned by the class. Destroy it in the destructor. Then you'll see a leak. – David Heffernan Mar 22 '18 at 21:44
3

The second Create call is on an instance, hence it operates on that instance and does not create yet another instance. [link](http://docwiki.embarcadero.com/RADStudio/en/Methods#Constructors). – Sertac Akyuz Mar 22 '18 at 21:44
OK, I guess you get away with it because you first call TObject.Create which does nothing. But still, you may as well instantiate with the virtual constructor – David Heffernan Mar 22 '18 at 21:58
Interesting: my working and tested solution gets downvotes and even a delete request (why?!), while a wrong solution gets edited (not by the author) to become working :) never mind. This answer was never meant to be a real solution, I knew there was a correct one. I just think, it's still interesting and should stay for educational purposes... @DavidHeffernan – maf-soft Mar 23 '18 at 07:32
There's a difference between getting the concept right, but messing up the code, as I did, and getting the code right but for the wrong concept, as you did. Yes this might work but isn't it best to do it right? Hence the voting. – David Heffernan Mar 23 '18 at 07:41
I'd still be interested in deeper info or more links, @SertacAkyuz link was already very interesting. I.e. one thing I'm still wondering about: how is the correct size of the new memory determined, when only the `TObject` constructor is called here? – maf-soft Mar 23 '18 at 07:48
Because the class reference that you pass is `ClassType` which is the class of `Self` – David Heffernan Mar 23 '18 at 09:54

Best way to implement Clone() in my specific classes

2 Answers2