How do I refactor this base class and split its functionality?

232 Views Asked by At

I have a base class TBuilder that inherits from TObjectList. TBuilder can do ADO related operations and populate its internal structure with the results. In addition to this, the same operation can be done over the web, via HTTP calls. The results returned is also parsed and the internal structure updated.

From here on I generate pas files from a database table to mimic its structure. Say I have a table called Company, I programmatically generate a TCompany object that also inherits from TBuilder which can then choose what it needs to be. At the moment I construct TCompany with a type that says I want it to do ADO operations or I want it to do HTTP operations. TBuilder would then typically have a load procedure and, based on the type, it will then generate SQL and load from the database (or http) and populate itself internally with the results.

Now what I'm trying to do now is to split TBuilder up so that one unit knows how to query the database via ADO and the other via HTTP. I was thinking to inherit these two classes from TBuilder as well. But I'm challanged with TCompany because it needs to inherit from either TBuilder, or TADOBuilder or TDSBuilder ( the latter two being the new units). If I inherit from TADOBuilder it can only represent the one type of object. I'm trying to make it so that TCompany can be either of the two at any time. I saw that you can only implement multiple inheritance with Interfaces, but I'm new to this and haven't been able to figure out how I can redesign it so that my TCompany can be both types of objects.

Any ideas how I can approach this? For the time being I'm stuck in Delphi 6 doing this.

This is how it looks:

 TCompany = class(TBuilder) //I generate this programatically. This represents a table in the database
      private
        fUser: TSecurityUser;

        function GetCompanyName: TBuilderField;
        function GetCompanyAbbreviation: TBuilderField;
        function GetCompanyID: TBuilderField;
        function GetDateCreated: TBuilderField;
        function GetDeleted: TBuilderField;
      public
        Property CompanyID:TBuilderField read GetCompanyID;
        Property CompanyName:TBuilderField read GetCompanyName;
        Property Abbreviation:TBuilderField read GetCompanyAbbreviation;
        property DateCreated:TBuilderField read GetDateCreated;
        property Deleted:TBuilderField read GetDeleted;

        property User:TSecurityUser read fUser Write fUser;

        constructor NewObject(psCompanyName,psAbbreviation:string);
        constructor Create(conType:TConnectionType = conTypeSQLServer);override;

This is how the Load procedure looks like, at the moment that I'm trying to split out into seperate units, in a smarter way:

function TBuilder.Load(psSQL:string = ''; poLoadOptions:TLoadOptions = []; poConnectionType:TConnectionType = conNone): Boolean;
var
  LoQuery:TADOQuery;
  LoSQL:string;
  LoConnectionType:TConnectionType;
begin
  Result := True;

  LoConnectionType := fConnectionType;
  if poConnectionType <> conNone then
    LoConnectionType := poConnectionType;

  if LoConnectionType = conTypeSQLServer then
  begin
    LoQuery := TADOQuery.Create(nil);

    Try
      try
        LoQuery.Connection := uBuilder.ADOConnection;
        LoSQL := psSQL;
        if LoSQL = '' then
          LoSQL := BuildSelectSQL;
        LoQuery.SQL.Text := LoSQL;
        LoQuery.Open;
        LoadFromDataset(LoQuery);
      except on E:exception do
        begin
          Error := E.Message;
          Result := False;
        end;
      end;
    Finally
      FreeAndNil(LoQuery);
    end;
  end else
  if fConnectionType = conTypeDatasnap then
  begin

    fWebCallType := sqlSelect;
    try

      if Assigned(fParent) then
        if fParent.Error <> '' then
          Exit;

      if Assigned(OnBusyLoadingHook) then
        OnBusyLoadingHook('Busy loading...');

      {Reset the msg}
      if Assigned(OnDisplayVisualError) then
        OnDisplayVisualError(imtRed,'');

      if (poLoadOptions <> LoadOptions) then
        LoadOptions := LoadOptions + poLoadOptions;
      Result := InternalLoad(loFullyRecursiveLoad in LoadOptions);
    finally
      { We're done loading }
      if Assigned(OnBusyLoadingHook) then
        OnBusyLoadingHook('');
    end;

  end;
end;

Then I'd use the object in this way:

var
  LoCompany:TCompany;
begin
  LoCompany := TCompany.Create(conTypeDatasnap);
  LoCompany.CompanyName.Value := 'Test';
  LoCompany.Load; //This will then generate the appropriate JSON and pass it via idhttp component to the server and the results will be parsed into its structure.  
end;

If I change the type it will query the database directly.

1

There are 1 best solutions below

10
RM. On

Option 1)

Do not inherit TCompany from TBuilder. Add an FBuilder: TBuilder field/property to TCompany and set this to either a TADOBuilder or TDSBuilder instance. And then add the required methods to TCompany and those methods would need to call the required method on FBuilder. Of course the methods needed has to be declared as virtual on TBuilder and TADOBuilder would need to override those.

Option2)

Separate your business object (TCompany) from the persistence code (TBuilder, TADOBuilder). Hard to give concrete advice without knowing the details but the idea is that your TCompany should be independent from the persistence code. In general you add all the required business properties to TCompany (e.g. Name, Address) and use a separate class which loads data into a TCompany using a TBuilder or TADOBuilder etc.

EDIT

Here is how it would look like with Option1.

TBuilder = abstract class
  procedure Load; virtual;
end;

TADOBuilder = class(TBuilder)
  procedure Load; override;
end;

TDSBuilder = class(TBuilder)
  procedure Load; override;
end;

TCompany = class
private
  FBuilder: TBuilder;
public
  constructor Create(aBuilder: TBuilder);
  procedure Load;
end;

{ TCompany }

constructor TCompany.Create(aBuilder: TBuilder);
begin
  inherited;
  FBuilder := aBuilder;
end;

procedure TCompany.Load;
begin
  FBuilder.Load;
end;

....

EDIT example for Option 2

TCompany = class
private
  FId: Integer;
  FName: string;
...
public
  property Id: Integer read FId write FId;
  property Name: string read FName write FName;
end;

TADOPerssiter = class
public
  constructor Create(const aConnectionString: string);
  // Creates and loads TCompany from ADO
  function LoadCompany(aId: Integer): TCompany;
  procedure SaveCompany(aCompany: TCompany);
end;

Add similar class for DS