Delphi: Maintainability Virtual vs Virtual Abstract

Question

I was writing a bunch of code a few months ago and now I'm adding stuff to it. I realized I wrote a bunch of functions that descend from a class that has about 2/3rds of its functions abstract and the remaining 1/3rd virtual.

I'm pretty much sick of seeing:

function descendent.doSomething() : TList;
begin
   inherited;
end;

when I've got this for the base class:

function descendent.doSomething() : TList;
begin
   result := nil;
end;

and would hate to wind up with:

function descendent.doSomething() : TList;
begin

end;

and then wonder why something didn't work.

I like using abstract functions because the compiler will let you know if you're liable to get an abstract error because you didn't implement some functions.

My question is, because I'm still a relatively new Delphi programmer and I've never had to maintain anything 8 years hence, is it worth taking the time to prune your code in this manner (i.e. remove functions that just have inherited in them and change your base class functions from abstract to concrete)

Answer 1

It depends on the problem as always. I use interfaces to define the user interface for the set of classes. At least when I know I will have more than one implementation of the underlying actual class. For instance You can have something like this:

 IAllInterfaced = interface(IInterface)
    procedure ImplementMeEverywhere_1(const Params: TParams);
    procedure ImplementMeEverywhere_2(const Params: TParams);
    procedure ImplementMeEverywhere_3(const Params: TParams);
  end;

  TAllInterfaced_ClassA = class(TInterfacedObject, IAllInterfaced)
  public
    procedure ImplementMeEverywhere_1(const Params: TParams);
    procedure ImplementMeEverywhere_2(const Params: TParams);
    procedure ImplementMeEverywhere_3(const Params: TParams);
  end;

  TAllInterfaced_ClassB = class(TInterfacedObject, IAllInterfaced)
  public
    procedure ImplementMeEverywhere_1(const Params: TParams);
    procedure ImplementMeEverywhere_2(const Params: TParams);
    procedure ImplementMeEverywhere_3(const Params: TParams);
  end;

Here you dont have a common ancestor. Each class only implements the interface and has no common underlying structure in a form of a common base class. This is possible if implementations are so different that they do not share anything, but he interface itself. You still need to use the same interface so you are consistent towards the users of the derived classes.

The second option is:

 IAllAbstract = interface(IInterface)
    procedure ImplementMeEverywhere_1(const Params: TParams);
    procedure ImplementMeEverywhere_2(const Params: TParams);
    procedure ImplementMeEverywhere_3(const Params: TParams);
  end;

  TAllAbstract_Custom = (TInterfacedObject, IAllAbstract)
  private
    ...
  public
    procedure ImplementMeEverywhere_1(const Params: TParams); virtual; abstract;
    procedure ImplementMeEverywhere_2(const Params: TParams); virtual; abstract;
    procedure ImplementMeEverywhere_3(const Params: TParams); virtual; abstract;
  end;

  TAllAbstract_ClassA = class(TAllAbstract_Custom)
  public
    procedure ImplementMeEverywhere_1(const Params: TParams); override;
    procedure ImplementMeEverywhere_2(const Params: TParams); override;
    procedure ImplementMeEverywhere_3(const Params: TParams); override;
  end;

  TAllAbstract_ClassB = class(TAllAbstract_Custom)
  public
    procedure ImplementMeEverywhere_1(const Params: TParams); override;
    procedure ImplementMeEverywhere_2(const Params: TParams); override;
    procedure ImplementMeEverywhere_3(const Params: TParams); override;
  end;

Here you have a base class for all the classes. In that class you can have common properties or event other classes etc... But all procedures are marked as abstract because they do not perform any common tasks. Abstract ensures the they will be implemented in the derived classes, but you do not need to implement "FieldA" in every class, you only implement it in the "TAllAbstract_Custom". This ensures that the DRY principle is used.

The last option is:

 IAllVirtual = interface(IInterface)
    procedure ImplementMeEverywhere_1(const Params: TParams);
    procedure ImplementMeEverywhere_2(const Params: TParams);
    procedure ImplementMeEverywhere_3(const Params: TParams);
  end;

  TAllVirtual_Custom = (TInterfacedObject, IAllVirtual)
  private
    ...
  public
    procedure ImplementMeEverywhere_1(const Params: TParams); virtual;
    procedure ImplementMeEverywhere_2(const Params: TParams); virtual;
    procedure ImplementMeEverywhere_3(const Params: TParams); virtual;
  end;

  TAllVirtual_ClassA = class(TAllVirtual_Custom)
  public
    procedure ImplementMeEverywhere_1(const Params: TParams); override;
    procedure ImplementMeEverywhere_2(const Params: TParams); override;
    procedure ImplementMeEverywhere_3(const Params: TParams); override;
  end;

  TAllVirtual_ClassB = class(TAllVirtual_Custom)
  public
    procedure ImplementMeEverywhere_1(const Params: TParams); override;
    procedure ImplementMeEverywhere_2(const Params: TParams); override;
    procedure ImplementMeEverywhere_3(const Params: TParams); override;
  end;

Here all derived classes have a common base virtual procedure. This ensures you do not have to implement every single procedure at the level of the derived classes. You can only override some parts of the code or none at all.

Naturally this are all edge cases, there is room in beetwen them. You can have a mix of those concepts.

Just remember:

1. Interfaces are powerfull tool to ensure that you hide implementation from the user and that you have a common usage point (interface). They also force some norms to be used, because interface needs to be implemented in full.
2. Abstract is a good tool so you do not have to use empty stubs for procedures where there is no real need for them. On the other side they force you to implement them in derived classes.
3. Virtual comes in handy when you have common code that must be implemented by every class and that ensures the clean OP and DRY principle. They are also welcome when you have procedures that not every derived class has or needs.

Sorry for a long answer but I could not give an easy explanation here because there is none. It all depends on the problem at hand. It is a balance between how much do the derived classes have in common and how different their implementations are.

Answer 2

If the code is really simple and you find it difficult to read and error-prone, then it's probably difficult to read and error-prone. (On the other hand, if the code is complicated and you find it hard to read, it could be your lack of experience. But not something like this.) You'd probably do well to refactor it now, while the issue is still fresh in your mind.

Answer 3

Yes, prune your code.

It makes your other code much easier to read (as you already mentioned, it would be easier to see what methods are actually overwritten). As an added benefit it will be easier to change the signature of the method in the parent class: Imagine you decide to pass one more parameter to a virtual method; You make the change to the parent class, then you'll need to repeat that same change for every child class that inherits from the given parent class. At that point you don't want bogus overwritten methods that just call "inherited"!