Is it the smell of code if an object knows a lot of its owner?

Question

In our Delphi 2007 application we use many of the following constructs

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

FindOwnerClass moves the owner hierarchy of the current component up to find a specific class (in the TdmBasicData example). The resulting object is stored in the FdmBasic field variable. We use this primarily for transmitting datamodules.

Example: When generating a report, the resulting data is compressed and stored in the Blob field of the table, accessible through the datamodule TdmReportBaseData. In a separate module of our application, there is a function that displays data from a report in Paged form using ReportBuilder. The main code of this module (TdmRBReport) uses the TRBTempdatabase class to convert compressed blob data into different tables that can be used in the Reportbuilder runtime report scheduler. TdmRBReport has access to TdmReportBaseData for all types of data related to the report (report type, report calculation settings, etc.). TRBTempDatabase is created in TdmRBReport, but has access to TdmReportBasedata. So now this is done using the construct above:

 constructor TRBTempDatabase.Create(aOwner: TComponent); begin inherited Create(aOwner); FdmReportBaseData := TdmRBReport(FindOwnerClass(Owner, TdmRBReport)).dmReportBaseData; end;{- .Create } 

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

What do you think about it? Is that the smell of code? If so, which way is better?

Answer 1

In the description presented here, I see this as slightly smelly. However, this is easy to fix.

I would be inclined to pass the dmReportBaseData object to the constructor of any component that it needs. This makes the contract clear at compile time, rather than forcing it at run time, as you do.

As at present, the contract you are fulfilling is stronger than necessary. Although TRBTempDatabase only requires an instance of dmReportBaseData , it will only work if it can get this instance from the TdmRBReport report TdmRBReport .

Performing this change will also allow TRBTempDatabase and TdmRBReport successfully work with the TRBTempDatabase and still. And, as @Lieven points out in the comments, this is likely to make testing easier.

Answer 2

If everything you do in the base class maintains a reference to the parent object, then no, this is not a code smell, this is completely legal use. You can explicitly create a base class to carry information about what may happen later.

If a base class relies on some characteristic of a derived class that is not present on its own (i.e., a generalized class relies on the specialization of one of its children), then yes, it can be a little funky.