Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

Given two interface definitions ...

IOmniWorkItem = interface ['{3CE2762F-B7A3-4490-BF22-2109C042EAD1}']
  function  GetData: TOmniValue;
  function  GetResult: TOmniValue;
  function  GetUniqueID: int64;
  procedure SetResult(const value: TOmniValue);
//
  procedure Cancel;
  function  DetachException: Exception;
  function  FatalException: Exception;
  function  IsCanceled: boolean;
  function  IsExceptional: boolean;
  property Data: TOmniValue read GetData;
  property Result: TOmniValue read GetResult write SetResult;
  property UniqueID: int64 read GetUniqueID;
end;

IOmniWorkItemEx = interface ['{3B48D012-CF1C-4B47-A4A0-3072A9067A3E}']
  function  GetOnWorkItemDone: TOmniWorkItemDoneDelegate;
  function  GetOnWorkItemDone_Asy: TOmniWorkItemDoneDelegate;
  procedure SetOnWorkItemDone(const Value: TOmniWorkItemDoneDelegate);
  procedure SetOnWorkItemDone_Asy(const Value: TOmniWorkItemDoneDelegate);
//
  property OnWorkItemDone: TOmniWorkItemDoneDelegate read GetOnWorkItemDone write SetOnWorkItemDone;
  property OnWorkItemDone_Asy: TOmniWorkItemDoneDelegate read GetOnWorkItemDone_Asy write SetOnWorkItemDone_Asy;
end;

... what are your ideas of laying out class declaration that inherits from both of them?

My current idea (but I don't know if I'm happy with it):

TOmniWorkItem = class(TInterfacedObject, IOmniWorkItem, IOmniWorkItemEx)
strict private
  FData              : TOmniValue;
  FOnWorkItemDone    : TOmniWorkItemDoneDelegate;
  FOnWorkItemDone_Asy: TOmniWorkItemDoneDelegate;
  FResult            : TOmniValue;
  FUniqueID          : int64;
strict protected
  procedure FreeException;
protected //IOmniWorkItem
  function  GetData: TOmniValue;
  function  GetResult: TOmniValue;
  function  GetUniqueID: int64;
  procedure SetResult(const value: TOmniValue);
protected //IOmniWorkItemEx
  function  GetOnWorkItemDone: TOmniWorkItemDoneDelegate;
  function  GetOnWorkItemDone_Asy: TOmniWorkItemDoneDelegate;
  procedure SetOnWorkItemDone(const Value: TOmniWorkItemDoneDelegate);
  procedure SetOnWorkItemDone_Asy(const Value: TOmniWorkItemDoneDelegate);
public
  constructor Create(const data: TOmniValue; uniqueID: int64);
  destructor Destroy; override;
public //IOmniWorkItem
  procedure Cancel;
  function  DetachException: Exception;
  function  FatalException: Exception;
  function  IsCanceled: boolean;
  function  IsExceptional: boolean;
  property Data: TOmniValue read GetData;
  property Result: TOmniValue read GetResult write SetResult;
  property UniqueID: int64 read GetUniqueID;
public  //IOmniWorkItemEx
  property OnWorkItemDone: TOmniWorkItemDoneDelegate read GetOnWorkItemDone write SetOnWorkItemDone;
  property OnWorkItemDone_Asy: TOmniWorkItemDoneDelegate read GetOnWorkItemDone_Asy write SetOnWorkItemDone_Asy;
end;

As noted in answers, composition is a good approach for this example but I'm not sure it applies in all cases. Sometimes I'm using multiple inheritance just to split read and write access to some property into public (typically read-only) and private (typically write-only) part. Does composition still apply here? I'm not really sure as I would have to move the property in question out from the main class and I'm not sure that's the correct way to do it.

Example:

// public part of the interface interface
IOmniWorkItemConfig = interface
  function  OnExecute(const aTask: TOmniBackgroundWorkerDelegate): IOmniWorkItemConfig;
  function  OnRequestDone(const aTask: TOmniWorkItemDoneDelegate): IOmniWorkItemConfig;
  function  OnRequestDone_Asy(const aTask: TOmniWorkItemDoneDelegate): IOmniWorkItemConfig;
end;

// private part of the interface
IOmniWorkItemConfigEx = interface ['{42CEC5CB-404F-4868-AE81-6A13AD7E3C6B}']
  function  GetOnExecute: TOmniBackgroundWorkerDelegate;
  function  GetOnRequestDone: TOmniWorkItemDoneDelegate;
  function  GetOnRequestDone_Asy: TOmniWorkItemDoneDelegate;
end;

// implementing class
TOmniWorkItemConfig = class(TInterfacedObject, IOmniWorkItemConfig, IOmniWorkItemConfigEx)
strict private
  FOnExecute        : TOmniBackgroundWorkerDelegate;
  FOnRequestDone    : TOmniWorkItemDoneDelegate;
  FOnRequestDone_Asy: TOmniWorkItemDoneDelegate;
public
  constructor Create(defaults: IOmniWorkItemConfig = nil);
public //IOmniWorkItemConfig
  function  OnExecute(const aTask: TOmniBackgroundWorkerDelegate): IOmniWorkItemConfig;
  function  OnRequestDone(const aTask: TOmniWorkItemDoneDelegate): IOmniWorkItemConfig;
  function  OnRequestDone_Asy(const aTask: TOmniWorkItemDoneDelegate): IOmniWorkItemConfig;
public //IOmniWorkItemConfigEx
  function  GetOnExecute: TOmniBackgroundWorkerDelegate;
  function  GetOnRequestDone: TOmniWorkItemDoneDelegate;
  function  GetOnRequestDone_Asy: TOmniWorkItemDoneDelegate;
end;
share|improve this question
add comment

1 Answer

up vote 1 down vote accepted

Did you consider using composition here? My Delphi is horrible, but this should get you an idea:

TOmniWorkItemEx = class(IOmniWorkItemEx)
    //implement it
end;

TOmniWorkItem = class(TInterfacedObject, IOmniWorkItem)
//implement the whole interface
//and now compose:
strict private
  FHandlers          : TOmniWorkItemEx;
protected
  function GetHandlers: IOmniWorkItemEx;
public
  property Handlers:IOmniWorkItemEx read GetHandlers;
end;

This splits the code nicely. Technically, IOmniWorkItemEx (and TOmniWorkItemEx) would best be parametrized with the function type and would have more generic method names (and a more generic name in the first place), and would thus be reusable.

Update:

Well, according to SOLID, interfaces should be as small as possible and classes should only have one responsibility (having a class implement too many interfaces is often an indicator that it violates the SRP).

For code division and code reuse, there's two approaches:

  1. inheritance and partial implementation
  2. composition and decomposition

Composition can be tedious, especially since most languages don't support syntactic delegation, but other than that, it almost certainly leads to better results.

In your last example, you have some code duplication and lack of normalization as well. Here's how I'd do it:

IWorkerItem<Owner, Worker> = interface
    function GetWith(const aTask: Worker):Owner
    function GetWorker: Worker
end;

class TOmniConfigWorkerItem<Worker> = class(IWorkerItem<TOmniWorkItemConfig, Worker>)
    //... implement
end;

And use it like this:

private strict
   FOnExecute        : TOmniConfigWorkerItem<TOmniBackgroundWorkerDelegate>;
   FOnRequestDone    : TOmniConfigWorkerItem<TOmniWorkItemDoneDelegate>;
   FOnRequestDone_Asy: TOmniConfigWorkerItem<TOmniWorkItemDoneDelegate>;
protected
   function GetOnExecute        : TOmniConfigWorkerItem<TOmniBackgroundWorkerDelegate>;
   function GetOnRequestDone    : TOmniConfigWorkerItem<TOmniWorkItemDoneDelegate>;
   function GetOnRequestDone_Asy: TOmniConfigWorkerItem<TOmniWorkItemDoneDelegate>;
public
   property OnExecute : IWorkerItem<TOmniWorkItemConfig, TOmniBackgroundWorkerDelegate> read GetOnExecute; 
   property OnRequestDone : IWorkerItem<TOmniWorkItemConfig, TOmniBackgroundWorkerDelegate> read GetOnRequestDone; 
   property OnRequestDone_Asy : IWorkerItem<TOmniWorkItemConfig, TOmniBackgroundWorkerDelegate> read GetOnRequestDone_Asy; 

This probably doesn't look like much of an improvement. In languages where declaring a read-only property is a matter of one line, it becomes really apparent. But even in this case, you swapped two partial abstractions against a very simple, atomic one.

Also the code might not compile. Some languages demand the type of an accessor to be equal to that of its property, so you might need to change that. I just thought it'd be better to use the abstraction for the public interface and the concretisation for the internal implementation.

share|improve this answer
    
Good idea, but I'm not sure it applies to all my usage cases. I'll update the question ... –  gabr Nov 22 '11 at 8:52
    
Pursuing SOLID in this case is like pursuing database normalisation forms to the extreme - it makes no sense. –  gabr Nov 22 '11 at 17:35
    
@gabr: Well, that's your opinion and unless you care to provide an argumentation that goes a little further than just assuming your point and dismissing the converse as non-sense, I won't argue ;) –  back2dos Nov 22 '11 at 17:48
    
As you've said: "This probably doesn't look like much of an improvement." Any way, what I was asking about was a way to lay out the code in a nice manner, not to refactor it. –  gabr Nov 22 '11 at 18:47
    
Although I won't go the way you describe, this is a valid answer (and a very good point) and deserves to be selected as an official answer to my question. –  gabr Nov 23 '11 at 17:08
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.