10

In our Delphi 2007 application we are using a lot of the following constructs

FdmBasic:=TdmBasicData(FindOwnerClass(AOwner,TdmBasicData));

The FindOwnerClass travels the Owner hierarchy of the current component upwards to find a specific class (in the example TdmBasicData). The resulting object is stored in the Field variable FdmBasic. We use this primarily to pass datamodules along.

Example: When generating a report, the resulting data is compressed and stored in a Blob field of a table accessed through a datamodule TdmReportBaseData. In a separate module of our application, there is functionality to show the data from the report in a Paged form using ReportBuilder. The main code of this module (TdmRBReport), uses a class TRBTempdatabase to convert the compressed blob data into different tables that are usable in the Reportbuilder runtime reportdesigner. TdmRBReport has access to TdmReportBaseData for all kinds of report-related data (type of report, report calculationsettings, etc). TRBTempDatabase is constructed in TdmRBReport but has to have access to TdmReportBasedata. So this is now done using the construction above:

constructor TRBTempDatabase.Create(aOwner: TComponent);
begin
  inherited Create(aOwner);

  FdmReportBaseData := TdmRBReport(FindOwnerClass(Owner, TdmRBReport)).dmReportBaseData;
end;{- .Create }

My feeling is that this means that TRBTempDatabase knows a lot of its owner, and I was wondering if this is some sort of code smell or Anti-pattern.

What are your thoughts about this? Is this a code smell? If so, what is a better way?

1
  • 1
    If it were supposed to know that much about another class it would have been provided an easier way of doing it. Feb 6, 2012 at 3:50

2 Answers 2

14

This kind of looks like a Service Locator Pattern which was first described by Martin Fowler (which has been identified as a common anti-pattern).

Construction based Dependency Injection is preferred over a Service Locator as it promotes visibility of the require parameters and promotes simpler Unit Testing.

The problem with using a Service Locator isn’t that you take a dependency on a particular Service Locator implementation (although that may be a problem as well), but that it’s a bona-fide anti-pattern. It will give consumers of your API a horrible developer experience, and it will make your life as a maintenance developer worse because you will need to use considerable amounts of brain power to grasp the implications of every change you make.

The compiler can offer both consumers and producers so much help when Constructor Injection is used, but none of that assistance is available for APIs that rely on Service Locator.

Also most notably it also breaks the Law of Demeter

The Law of Demeter (LoD) or Principle of Least Knowledge is a design guideline for developing software, particularly object-oriented programs. In its general form, the LoD is a specific case of loose coupling.

Law of Demeter for functions requires that a method M of an object O may only invoke the methods of the following kinds of objects:

  1. O itself
  2. M's parameters
  3. any objects created/instantiated within M
  4. O's direct component objects
  5. a global variable, accessible by O, in the scope of M

In particular, an object should avoid invoking methods of a member object returned by another method. For many modern object oriented languages that use a dot as field identifier, the law can be stated simply as "use only one dot". That is, the code a.b.Method() breaks the law where a.Method() does not. As a simple example, when one wants to walk a dog, it would be folly to command the dog's legs to walk directly; instead one commands the dog and lets it take care of its own legs.

The Better Way

Effectively the better way is to remove the service locator call within the class, and pass in the correct owner as a parameter inside it's constructor. Even if this means that you have a service class which performs an owner look up and then passes that to the class constructor

constructor TRBTempDatabase.Create(aOwner: TComponent, ownerClass: IComponent);
begin
  inherited Create(aOwner);

  FdmReportBaseData := TdmRBReport(ownerClass).dmReportBaseData;
end;{- .Create }
1
  • 3
    Great answer, and props to whoever came up with this wonderful analogy: As a simple example, when one wants to walk a dog, it would be folly to command the dog's legs to walk directly; instead one commands the dog and lets it take care of its own legs.
    – Andy Hunt
    Aug 12, 2011 at 9:13
3

One of the difficulties with having child objects knowing too much about the parent is that you end up implementing patterns that can (and most often do) become too tightly coupled, which creates major dependency headaches and often later becomes very difficult to modify & maintain safely later on.

Depending on how deeply your two classes are connected, it sounds a bit like Fowler's description of the Feature Envy or Inappropriate Intimacy code smells are evident.

There appears to be need to load or read a class with data, in which case you could use a number of alternate patterns to break the dependency between the child and it's chain of parents, and it sounds like you need to delegate the task of accessing your data class rather than making the data accessor class responsible for doing everything itself.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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