Legacy Code: Abstract to Interface

Say you where given the following legacy code snippet, and where told that you needed five new  packet types that need to be written.

   TDataObject = class(TObject)
     // Some Shared Data here.
   end;

   TPacketWriter = class(TObject)
   private
      function CreateObjectX : TDataObject;
      procedure WriteFileZ(Data : TDataObject);
   protected
      procedure WriteFileX(Data : TDataObject); virtual; abstract;
      procedure WriteFileY(Data : TDataObject); virtual; abstract;
   public
      procedure Execute;
   end;

   TPacketWriterTypeA = class(TPacketWriter)
   protected
      procedure WriteFileX(Data : TDataObject); override;
      procedure WriteFileY(Data : TDataObject); override;
   end;

   TPacketWriterTypeB = class(TPacketWriter)
   protected
      procedure WriteFileX(Data : TDataObject); override;
      procedure WriteFileY(Data : TDataObject); override;
   end;

   ...
   
procedure TPacketWriter.Execute;
var
 lObj : TDataObject;
begin
  lObj := CreateObjectX;
  try
    WriteFileX(lObj);
    WriteFileY(lObj);
    WriteFileZ(lObj);
  finally
    lObj.Free;
  end;
end;

  ...
  // where it was used
var
 PWA : TPacketWriterTypeA;
 PWB : TPacketWriterTypeB;
begin
 PWA := TPacketWriterTypeA.create;
 try
   PWA.Execute;
 finally
   PWA.Free;
 end;

 PWB := TPacketWriterTypeA.create;
 try
   PWB.Execute;
 finally
   PWB.Free;
 end;
end;

Well you might say that is easy I can quickly add the following.

   TPacketWriterNewType1 = class(TPacketWriter)
   protected
      procedure WriteFileX(Data : TDataObject); override;
      procedure WriteFileY(Data : TDataObject); override;
   end;

   TPacketWriterNewType2 = class(TPacketWriter)
   protected
      procedure WriteFileX(Data : TDataObject); override;
      procedure WriteFileY(Data : TDataObject); override;
   end;

   TPacketWriterNewType3 = class(TPacketWriter)
   protected
      procedure WriteFileX(Data : TDataObject); override;
      procedure WriteFileY(Data : TDataObject); override;
   end;

   TPacketWriterNewType4 = class(TPacketWriter)
   protected
      procedure WriteFileX(Data : TDataObject); override;
      procedure WriteFileY(Data : TDataObject); override;
   end;

   TPacketWriterNewType5 = class(TPacketWriter)
   protected
      procedure WriteFileX(Data : TDataObject); override;
      procedure WriteFileY(Data : TDataObject); override;
   end;

But I have goal that all new code should be unit tested. Let just say that WriteFileX and WriteFileY are unique in each case and they are complex pieces of code. In the real world example The Execute Method was even more complex. Writing a test on just the execute method would involve a lot of cut and paste of test code for each instance. Not something that smelled correct to me.

The key here that I want to point out is that when you see virtual; abstract; you should ask if an interface makes sense. In this case the answer I determined that an interface made sense. I refactored the TPacketWriter to implement an interface with the resulting code looking like this:

   TDataObject = class(TObject)
     // Some Shared Data here.
   end;

   IPacketFileWriter = interface
     ['{9BE43D5A-5566-4218-A83A-A54D567AD986}']
     procedure WriteFileX(Data : TDataObject);
     procedure WriteFileY(Data : TDataObject);
   end;

   TPacketWriter = class(TObject)
   private
      function CreateObjectX : TDataObject;
      procedure WriteFileZ(Data : TDataObject);
   public
      procedure Execute(aFileWriter : IPacketFileWriter);
   end;

   TPacketWriterTypeA = class(TInterfacedObject,IPacketFileWriter)
   protected
      procedure WriteFileX(Data : TDataObject);
      procedure WriteFileY(Data : TDataObject);
   end;

   TPacketWriterTypeB = class(TInterfacedObject,IPacketFileWriter)
   protected
      procedure WriteFileX(Data : TDataObject);
      procedure WriteFileY(Data : TDataObject);
   end;

   TPacketWriterNewType1 = class(TInterfacedObject,IPacketFileWriter)
   protected
      procedure WriteFileX(Data : TDataObject);
      procedure WriteFileY(Data : TDataObject);
   end;

   TPacketWriterNewType2 = class(TInterfacedObject,IPacketFileWriter)
   protected
      procedure WriteFileX(Data : TDataObject);
      procedure WriteFileY(Data : TDataObject);
   end;

   TPacketWriterNewType3 = class(TInterfacedObject,IPacketFileWriter)
   protected
      procedure WriteFileX(Data : TDataObject);
      procedure WriteFileY(Data : TDataObject);
   end;

   TPacketWriterNewType4 = class(TInterfacedObject,IPacketFileWriter)
   protected
      procedure WriteFileX(Data : TDataObject);
      procedure WriteFileY(Data : TDataObject);
   end;

   TPacketWriterNewType5 = class(TInterfacedObject,IPacketFileWriter)
   protected
      procedure WriteFileX(Data : TDataObject);
      procedure WriteFileY(Data : TDataObject);
   end;

...

procedure TPacketWriter.Execute(aFileWriter : IPacketFileWriter);
var
 lObj : TDataObject;
begin
  lObj := CreateObjectX;
  try
    aFileWriter.WriteFileX(lObj);
    aFileWriter.WriteFileY(lObj);
    WriteFileZ(lObj);
  finally
    lObj.Free;
  end;
end;

...

// Where it was used
var
 PW : TPacketWriter;
begin
  PW := TPacketWriter.Create;
  try
    PW.Execute(TPacketWriterTypeA.Create);
    PW.Execute(TPacketWriterTypeB.Create);
    PW.Execute(TPacketWriterNewType1.Create);
    PW.Execute(TPacketWriterNewType2.Create);
    PW.Execute(TPacketWriterNewType3.Create);
    PW.Execute(TPacketWriterNewType4.Create);
    PW.Execute(TPacketWriterNewType5.Create);
  finally
    PW.Free;
  end;
end;

So why do this?

1. I was able to write a test for TPacketWriter.execute using a Mock Object that implemented the  Interface
2. I was able to test both WriteFilesX and WriteFileY for each class independently 
3. Implementing new supporting types is easy and I don't have to worry about TPacketWriter.Execute when I am writing my test.

I am sure there are other reasons, but writing testable code was my primary motivation.

There are many other things I can change with this code.  But the point I want to repeat.

When you see virtual; abstract; you should ask if an interface makes sense.