13

I'm trying to figure out the best way to use dependency injection for some legacy code which will take a long to refactor and has to be done gradually. Most of the old classes use a "Parent" property for determining various things and the parent property was often pass in via a constructor argument as follows:

constructor TParentObject.Create;
begin
  FChildObject := TChildObject.Create(Self);
end;

constructor TChildObject.Create(AParent: TParentObject)
begin
  FParent := AParent;
end;

This is fairly typical of our legacy code base. However when moving to interfaces and constructor injection, the Parent is not known by the Spring4D framework when creating the Child object. So it will just get a new parent but not the existing one. Of course I can create a property getter/setter but this would indicate an "optional" property for the class which is really a mandatory property. See the code below for more explanation:

unit uInterfaces;

interface

uses
  Spring.Collections;

type

  IChildObject = interface;

  IParentObject = interface
  ['{8EA8F9A2-E627-4546-8008-0A77DA2B16F1}']
    function GetSomethingRequiredByChild: string;
    procedure SetSomethingRequiredByChild(const Value: string);
    property SomethingRequiredByChild: string read GetSomethingRequiredByChild write SetSomethingRequiredByChild;
    function GetChild: IChildObject;
    property Child: IChildObject read GetChild;
  end;

  // This introduces a property getter/setter
  // However it also implies that Parent can be NIL which it cannot
  IChildObject = interface
  ['{ECCA09A6-4A52-4BE4-A72E-2801160A9086}']
    function GetParent: IParentObject;
    procedure SetParent(const Value: IParentObject);
    property Parent: IParentObject read GetParent write SetParent;
  end;

  TParentObject = class(TInterfacedObject, IParentObject)
  private
    FChild: IChildObject;
    FSomethingRequiredByChild: string;
    function GetChild: IChildObject;
    function GetSomethingRequiredByChild: string;
    procedure SetSomethingRequiredByChild(const Value: string);
  public
    constructor Create;
  end;

  TChildObject = class(TInterfacedObject, IChildObject)
  private
    FParent: IParentObject;
    function GetParent: IParentObject;
    procedure SetParent(const Value: IParentObject);
  public
    // This requries a Parent object, but how does the Spring4D resolve the correct parent?
    constructor Create(const AParent: IParentObject);
  end;

implementation

uses
  Spring.Services;

{ TParentObject }

constructor TParentObject.Create;
begin
  // Here is the old way...
  FChild := TChildObject.Create(Self); // Old way of doing it

  // This is the Service Locator way...
  FChild := ServiceLocator.GetService<IChildObject>;
  // I would prefer that the Parent is assigned somehow by the Service Locator
  // IS THIS POSSIBLE - or am I dreaming?
  FChild.Parent := Self;
end;

function TParentObject.GetChild: IChildObject;
begin
  Result := FChild;
end;

function TParentObject.GetSomethingRequiredByChild: string;
begin
  Result := FSomethingRequiredByChild;
end;

procedure TParentObject.SetSomethingRequiredByChild(const Value: string);
begin
  FSomethingRequiredByChild := Value;
end;

{ TChildObject }

constructor TChildObject.Create(const AParent: IParentObject);
begin
  FParent := AParent;
end;

function TChildObject.GetParent: IParentObject;
begin
  Result := FParent;
end;

procedure TChildObject.SetParent(const Value: IParentObject);
begin
  FParent := Value;
end;

end.

Maybe there is some methodology which can be used that I'm not aware of to set the parent object using the DI framework?

I hope this question is clear what I'm trying to achieve. I'm happy to provide more description/code example where necessary.

Rick Wheeler
  • 1,142
  • 10
  • 22
  • It would be interesting to see the code that actually "creates" the child objects. I smell service locator. – Stefan Glienke Aug 11 '15 at 05:46
  • Haha Stefan you are indeed correct that's what I was planning to use but wondered if there is an alternative? I'll post the code now. – Rick Wheeler Aug 11 '15 at 05:54
  • 1
    My question would be: why do you feel this is any better than directly calling the constructor of TChildObject in TParentObject? I guess the real code is a bit more complex but still: if you have a parent/child relation the classes might know each other anyway. If that is not the case I would suggest using the factory pattern. Will post some code. – Stefan Glienke Aug 11 '15 at 06:04
  • I'm trying to decouple many units of old "spaghetti" code and make them small, clean class with interfaces in separate units so we can create unit tests for all classes. We prefer to remove as many tightly coupled dependencies as possible and let the framework deal with that via injection. It works perfectly for new code we are writing, however to migrate legacy code is more challenging :-) – Rick Wheeler Aug 11 '15 at 06:08
  • 1
    If you want to remove this parent/child coupling, can you just do that? Can the child exist without its parent? Or does the parent need to outlive the child? – David Heffernan Aug 11 '15 at 07:23
  • @DavidHeffernan currently the child needs to know things from the parent so it can behave correctly. Maybe after many more code revisions & extensive refactoring this can be changed, but for now the relationship must be there. – Rick Wheeler Aug 12 '15 at 03:17

1 Answers1

16

First of all you should not use the service locator to replace ctor calls. This is just making things worse. I know people think they are smart by doing that but really you are replacing one simple dependency on another class with a dependency on some global state plus the requirement that some other code out of (the consuming classes) control puts the dependency into the container. That does not result in easier but harder to maintain code.

Plus all the other reasons why you should stay away from it. The service locator might have a limited use in legacy application to introduce a composition root in the middle of an application to start DI from that point on but not in a way you show.

If parent needs the child then just inject it. Now the problem is if you want to create a parent, you first need the child but the child needs the parent. How to achieve that? There are two solutions. However one of them is not pure DI compatible.

I first show the way using a factory provided by the container (needs latest develop branch version as of the time of posting):

unit ParentChildRelationShip.Types;

interface

uses
  SysUtils,
  Spring,
  Spring.Container.Common;

type
  IChildObject = interface;

  IParentObject = interface
    ['{8EA8F9A2-E627-4546-8008-0A77DA2B16F1}']
    function GetChild: IChildObject;
    property Child: IChildObject read GetChild;
  end;

  IChildObject = interface
    ['{ECCA09A6-4A52-4BE4-A72E-2801160A9086}']
    function GetParent: IParentObject;
    property Parent: IParentObject read GetParent;
  end;

  TParentObject = class(TInterfacedObject, IParentObject)
  private
    FChild: IChildObject;
    function GetChild: IChildObject;
  public
    constructor Create(const childFactory: IFactory<IParentObject, IChildObject>);
  end;

  TChildObject = class(TInterfacedObject, IChildObject)
  private
    FParent: WeakReference<IParentObject>;
    function GetParent: IParentObject;
  public
    constructor Create(const AParent: IParentObject);
  end;

implementation

{ TParentObject }

constructor TParentObject.Create;
begin
  FChild := childFactory(Self);
end;

function TParentObject.GetChild: IChildObject;
begin
  Result := FChild;
end;

{ TChildObject }

constructor TChildObject.Create(const AParent: IParentObject);
begin
  FParent := AParent;
end;

function TChildObject.GetParent: IParentObject;
begin
  Result := FParent;
end;

end.

program ParentChildRelation;

{$APPTYPE CONSOLE}

uses
  SysUtils,
  Spring.Container,
  Spring.Container.Common,
  ParentChildRelationShip.Types in 'ParentChildRelationShip.Types.pas';

procedure Main;
var
  parent: IParentObject;
  child: IChildObject;
begin
  GlobalContainer.RegisterType<IParentObject,TParentObject>;
  GlobalContainer.RegisterType<IChildObject,TChildObject>;
  GlobalContainer.RegisterFactory<IFactory<IParentObject,IChildObject>>(TParamResolution.ByValue);
  GlobalContainer.Build;
  parent := GlobalContainer.Resolve<IParentObject>;
  child := parent.Child;
  Assert(parent = child.Parent);
end;

begin
  try
    Main;
  except
    on E: Exception do
      Writeln(E.Message);
  end;
  ReportMemoryLeaksOnShutdown := True;
end.

If you don't want to use a container provided factory you explicitly register it yourself. Then the RegisterFactory call is replaced with this one:

  GlobalContainer.RegisterInstance<TFunc<IParentObject,IChildObject>>(
    function(parent: IParentObject): IChildObject
    begin
      Result := GlobalContainer.Resolve<IChildObject>([TValue.From(parent)]);
    end);

And the constructor parameter can be changed to TFunc<...> as it does not need RTTI for this method (that is why you needed IFactory<...> in the other case).

The second version uses field injection and thus is pure DI incompatible - be careful writing code like that as it does not work without using the container or RTTI - like if you want to test these classes it might become hard to compose them without the container. The important part here is the PerResolve which tells the container to reuse the once resolved instance whenever another dependency is needed that it can satisfy.

unit ParentChildRelationShip.Types;

interface

uses
  SysUtils,
  Spring;

type
  IChildObject = interface;

  IParentObject = interface
    ['{8EA8F9A2-E627-4546-8008-0A77DA2B16F1}']
    function GetChild: IChildObject;
    property Child: IChildObject read GetChild;
  end;

  IChildObject = interface
    ['{ECCA09A6-4A52-4BE4-A72E-2801160A9086}']
    function GetParent: IParentObject;
    property Parent: IParentObject read GetParent;
  end;

  TParentObject = class(TInterfacedObject, IParentObject)
  private
    [Inject]
    FChild: IChildObject;
    function GetChild: IChildObject;
  end;

  TChildObject = class(TInterfacedObject, IChildObject)
  private
    FParent: WeakReference<IParentObject>;
    function GetParent: IParentObject;
  public
    constructor Create(const AParent: IParentObject);
  end;

implementation

function TParentObject.GetChild: IChildObject;
begin
  Result := FChild;
end;

{ TChildObject }

constructor TChildObject.Create(const AParent: IParentObject);
begin
  FParent := AParent;
end;

function TChildObject.GetParent: IParentObject;
begin
  Result := FParent;
end;

end.

program ParentChildRelation;

{$APPTYPE CONSOLE}

uses
  SysUtils,
  Spring.Container,
  Spring.Container.Common,
  ParentChildRelationShip.Types in 'ParentChildRelationShip.Types.pas';

procedure Main;
var
  parent: IParentObject;
  child: IChildObject;
begin
  GlobalContainer.RegisterType<IParentObject,TParentObject>.PerResolve;
  GlobalContainer.RegisterType<IChildObject,TChildObject>;
  GlobalContainer.Build;
  parent := GlobalContainer.Resolve<IParentObject>;
  child := parent.Child;
  Assert(parent = child.Parent);
end;

begin
  try
    Main;
  except
    on E: Exception do
      Writeln(E.Message);
  end;
  ReportMemoryLeaksOnShutdown := True;
end.

By the way. Watch your references between parent and child when using interfaces. If they reference each other you will get memory leaks. You can solve that by using a weak reference on one side (usually the parent reference in the child).

Stefan Glienke
  • 20,860
  • 2
  • 48
  • 102
  • Thanks for the extensive post Stefan - this Parent/Child would be really useful in the Spring4D examples. We are using DSharp.Mocks fairly extensively so will need to play around and decide which approach is best. I agree with your comment about using ServiceLocator to replace ctor's, but did not know what else to do. Is there any circumstance in which you think it is "ok" to use the ServiceLocator, or is it never ok? I noticed that you use GlobalContainer.Resolve instead of ServiceLocator - could you please describe the difference? Thanks again your continued community work is second to none. – Rick Wheeler Aug 12 '15 at 03:25
  • I'm trying to build the latest develop branch but it keeps failing: Spring.Container.CreationContext.pas(108): error E2033: Types of actual and formal var parameters must be identical – Rick Wheeler Aug 12 '15 at 04:37
  • 1
    Should work now. As for the service locator. I usually resolve from the container directly as the location where that happens is not deep into the application (i.e. composition root). I recommend against using the ServiceLocator - in a well designed architecture it does not have any place. In legacy application it might have a temporary place but as I said it might make things worse and not better. – Stefan Glienke Aug 12 '15 at 07:50