Interfaces - Time to face my ignorance.

[AlanScottAgain 1]

I know I should have done it sooner - I've been putting it off too long.

 

Thanks to PB for giving me a push to do this, in another tread.

So I've  stripped down my TDrawingObject and TDrawingObjectList Classes to show what I have and what I'm trying to acheive :

Type
  TDrawingObjectType = (doChild, doBackground);

 

 TDrawingObjectsList = Class; //Forward Declaration

 

  TDrawingObject = Class
  Private
    FObjectType: TDrawingObjectType;
    FDrawingObjectsList: TDrawingObjectsList;
  Public
    Constructor Create(AObjectType: TDrawingObjectType);
    Destructor Destroy;
    Property ObjectType: TDrawingObjectType Read FObjectType Write FObjectType;
    Property DrawingObjectsList: TDrawingObjectsList Read FDrawingObjectsList Write FDrawingObjectsList;
  End;

 

 

  TDrawingObjectsList = Class(TObjectList<TDrawingObject>)
  Private
    FOwnsObjects: Boolean;
  Protected
    Function GetItem(Index: Integer): TDrawingObject;
    Procedure SetItem(Index: Integer; AObject: TDrawingObject);
  Public
    Procedure Draw(ACanvas: IskCanvas);
    Property OwnsObjects: Boolean Read FOwnsObjects Write FOwnsObjects;
    Property Items[Index: Integer]: TDrawingObject Read GetItem Write SetItem; default;
    Procedure LoadDrawingObjects;
    
  End;

 

I'm trying to implement interfaces for these objects The code below looks pretty good at least I don't a lot of errors:

 

I have two main questions at the moment:

In the TDrawingListClass declaration  I used to have TDrawingObjectsList = Class(TObjectList<TDrawingObject>) to type cast the list. How do I achieve the same with the class/interface declaration?  TDrawingObjectsList = class(TInterfacedObject, IDrawingObjectList, TObjectList<TDrawingObject>) is not right.

In the class constructs I have kept the interface references eg: property DrawingObjectList: IDrawingObjectList read GetDrawingObjectList write SetDrawingObjectList;     Is this correct?

 

type
  TDrawingObjectType = (doChild, doBackground);

 

  IDrawingObjectList = interface; // forward declaration

 

  IDrawingObject = interface(IInterface)
    ['{A8701C68-51FC-4087-8C30-E3CA4C7C1D8E}']
    function GetDrawingObjectType: TDrawingObjectType;
    procedure SetDrawingObjectType(ADrawingObjectType: TDrawingObjectType);
    function GetDrawingObjectList: IDrawingObjectList;
    procedure SetDrawingObjectList(ADrawingObjectList: IDrawingObjectList);
    property ObjectType: TDrawingObjectType read GetDrawingObjectType write SetDrawingObjectType;
    property DrawingObjectList: IDrawingObjectList read GetDrawingObjectList write SetDrawingObjectList;
  end;

 

  TDrawingObject = class(TInterfacedObject, IDrawingObject)
    function GetDrawingObjectType: TDrawingObjectType;
    procedure SetDrawingObjectType(ADrawingObjectType: TDrawingObjectType);
    function GetDrawingObjectList: IDrawingObjectList;
    procedure SetDrawingObjectList(ADrawingObjectList: IDrawingObjectList);
    property ObjectType: TDrawingObjectType read GetDrawingObjectType write SetDrawingObjectType;
    property DrawingObjectList: IDrawingObjectList read GetDrawingObjectList write SetDrawingObjectList;

  end;

 

  IDrawingObjectList = interface(IInterface)
    ['{9D2C916D-19F0-49B2-93C6-B84592442057}']
    Private

    function GetOwnsObject: Boolean;
    procedure SetOwnsObject(AOwnsObject: Boolean);
    property OwnsObjects: Boolean read GetOwnsObject write SetOwnsObject;
    function GetItem(Index: Integer): IDrawingObject;
    procedure SetItem(Index: Integer; AObject: IDrawingObject);
    Public

    Constructor Create(AObjectType: TDrawingObjectType);
    Destructor Destroy;

    property Items[Index: Integer]: IDrawingObject read GetItem Write SetItem;

    procedure Draw(ACanvas: ISkCanvas);
    procedure LoadDrawingObjects;
  end;

 

  TDrawingObjectsList = class(TInterfacedObject, IDrawingObjectList)
  private
    FOwnsObjects: Boolean;
    function GetOwnsObject: Boolean;
    procedure SetOwnsObject(AOwnsObject: Boolean);
    property OwnsObjects: Boolean read GetOwnsObject write SetOwnsObject;
    function GetItem(Index: Integer): IDrawingObject;
    procedure SetItem(Index: Integer; AObject: IDrawingObject);
  public
    property Items[Index: Integer]: IDrawingObject read GetItem write SetItem;

    procedure Draw(ACanvas: ISkCanvas);
    procedure LoadDrawingObjects;
  end;

 

Thanks

Alan

Edited July 12, 2022 by AlanScottAgain

[Dalija Prasnikar 1396]

If you are having trouble managing objects memory, then using interfaces is a bad option as using them requires even more knowledge. Yes, they can manage memory for you, but there is so much fine print attached I wouldn't recommend it as a starting point.

 

Your drawing object list class also has some unnecessary code as it inherits from generic TObjectList that already automatically handles all that. I have simplified your code and added some basic workflow.

 

uses
  System.Generics.Collections;

type
  TDrawingObjectType = (doChild, doBackground);

  TDrawingObjectsList = class; //Forward Declaration

  TDrawingObject = class
  private
    FObjectType: TDrawingObjectType;
    FDrawingObjectsList: TDrawingObjectsList;
  public
    constructor Create(AObjectType: TDrawingObjectType);
    destructor Destroy; override;
    property ObjectType: TDrawingObjectType read FObjectType write FObjectType;
    property DrawingObjectsList: TDrawingObjectsList read FDrawingObjectsList;
  end;

  TDrawingObjectsList = class(TObjectList<TDrawingObject>)
  public
    procedure DrawDraw(ACanvas: IskCanvas);
    procedure LoadDrawingObjects;
  end;

  procedure BuildList(List: TDrawingObjectsList);

var
  Root: TDrawingObject;

implementation

constructor TDrawingObject.Create(AObjectType: TDrawingObjectType);
begin
  FDrawingObjectsList := TDrawingObjectsList.Create(True);
end;

destructor TDrawingObject.Destroy;
begin
  FDrawingObjectsList.Free;
  inherited;
end;

procedure BuildList(List: TDrawingObjectsList);
var
  Item: TDrawingObject;
begin
  List.Clear;

  // build list

  Item := TDrawingObject.Create(doChild);
  // set Item data
  // ...
  // Add Item to List
  List.Add(Item);

  Item := TDrawingObject.Create(doChild);
  // set Item data
  // ...
  // Add Item to List
  List.Add(Item);
end;


initialization

  Root := TDrawingObject.Create(doBackground);
  BuildList(Root.DrawingObjectsList);

finalization

  Root.Free;

end.

 

FDrawingObjectsList list will handle lifetime of any drawing object added to the list, so you don't have to deal with them. Please note that you cannot add same drawing object to multiple lists as it will be released twice, causing exception. Next, FDrawingObjectsList belongs to its drawing object instance and as such it should be constructed in constructor and destroyed in destructor of TDrawingObject. Nobody else should free that list nor assign anything to its reference there is no reason for it to be writeable property. Constructing/destroying part is the simplest and safest possible, there are other more complex coding patterns that involve lazy initialization, but they require more code and there is more room for bugs. Unless you need to preserve memory as much as possible, you don't need to deal with more complicated code. And if needed you can easily add that later on.

 

There is additional mistake in your destructor declaration. You are missing override directive. Without it destructor will never be called and you will have memory leaks.

 

I have created global Root object and populated it in initialization section. You don't have to make it a global, this is just to illustrate how to work with Root object and how to populate its data. BuildList takes TDrawingObjectsList as a parameter and populates existing list without constructing new one. if the list already has some items it will be cleared - you can omit clearing the list if you know you will populate it only once, or you can clear it in outside code before calling BuildList.

 

I am not going to comment on the interface part as it would be a really long write.

 

[Anders Melander 1784]

36 minutes ago, AlanScottAgain said:

I'm trying to implement interfaces for these objects

Why? What problem are you trying to solve?

 

While I'm definitely a proponent of interfaces at a higher level, at low level they often just obfuscate the code.

 

Apart from that I think your solution has a number of "smells":

• Why is the TDrawingObject.ObjectType writable? Does the ObjectType really need to change after construction?
• Why is TDrawingObject.DrawingObjectsList writable? Who owns the list? (might have been covered before. TL;DR)
• Instead of exposing the whole TObjectList<> class, expose only the relevant parts (Add, Remove, Enumerator). See:
• Why is TDrawingObjectsList.OwnsObjects part of the API? Isn't the ownership an implementation detail?

[hsauro 40]

Most articles will explain the properties of interfaces but rarely why and when you’d use them. Someone pointed me to this article (search for interfaces) and  it helped me a lot to understand why they can be useful.

 

http://www.unigui.com/doc/online_help/user-interface.htm?fbclid=IwAR12Yq3agChFOoUvH4OE8K9gHApA6y-p6NdIHejeRKSxjEmmEMwOhpP0CNw

[AlanScottAgain 1]

Thanks for all the feedback.

 

I think that interfaces at this stage is overkill.

 

As a result of the comments I have cleaned up the code and taken the advice on object lifetime and ownership.