Common mistakes when approaching OO design — Class dependencies

Dec 21 2006

Here we con­tinue with explain­ing some of the mis­takes com­monly made in Object Ori­ented design, and the good prac­tices that are often ignored. This arti­cle is focused on code main­tain­abil­ity and on improv­ing coöper­a­tion with peo­ple work­ing at the same project.

Encour­ag­ing class dependencies.

Hav­ing a lot of (mutual) depen­den­cies in the code is quite typ­i­cal of Spaghetti Code, and it’s def­i­nitely some­thing we want to avoid, in order to keep our design neat, improve main­tain­abil­ity and ensure ease of col­lab­o­ra­tion with col­leagues. What do I mean by “class depen­den­cies”? Let’s con­tinue the exam­ple from the last arti­cle, and sup­pose we have a cer­tain class GuiManager which, at some points, wants to gen­er­ate some reports. Let’s intro­duce now a cer­tain ReportManager, which is a class respon­si­ble for gen­er­at­ing reports. We have two types of report: TableReport, and ChartReport. They look like this:

class TableReport {
    public:
        void report()  {
            // do something
        }
};

class ChartReport {
    public:
        void report()  {
            // do something
        }
};

This means that the Report­Man­ager will have to look some­thing like this:

class ReportManager {
    public:
        void reportAll() {
            m_tableReport.report();
            m_chartReport.report();
        }

    private:
        TableReport m_tableReport;
        ChartReport m_chartReport;
};

There are sev­eral prob­lems in this imple­men­ta­tion. First of all, If the guy respon­si­ble for the TableReport one day wakes up, and decides that the method report() should rather be named generate(), he will not only be allowed to just change that and com­mit to the repos­i­tory, but this will break the ReportManager! So after a few hours, the guy respon­si­ble for the ReportManager checks out from the repos­i­tory, builds, and finds out that all the times he has used the TableReport need to be changed. Of course this is some­thing we don’t want to happen.

The usual approach to this, is using an Abstract Base Class (ABC), which is a very robust way to sort out prob­lems like this. Let’s see come code:

class Report {
    public:
        virtual void report() = 0;
};

class TableReport : public Report {
    public:
        void report()  {
            // do something
        }
};

class ChartReport : public Report {
    public:
        void report()  {
            // do something
        }
};

Report is our ABC, and with it we are lit­er­ally forc­ing the peo­ple who write TableReport and ChartReport to write a method named report(). So, this way we broke one depen­dency: the ReportManager doesn’t need to worry about the way every sin­gle report will call the method: it’s sure that a method named report() will exist.

There is, tho, another depen­dency. If some­body writes a new report, say XmlReport, this will need mod­i­fi­ca­tions to the ReportManager, because our logic so far implies that the ReportManager knows about all the reports. So, if we’re not the main­tain­ers of the ReportManager (because maybe it’s in some dif­fer­ent library, writ­ten by some­one else, and we don’t have access to the code), we will have to go ahead and ask the right­ful main­tainer to mod­ify the code. Hence, there’s an extra depen­dency, not struc­tural, this time, but log­i­cal. What if the main­tainer of the ReportManager gave us tools (read APIs) so that we can reg­is­ter our par­tic­u­lar report to the ReportManager? Con­sider the fol­low­ing code:

class ReportManager {
    public:
        void registerReport(Report const & r) {
            m_reports.push_back(r);
        }

        void reportAll() {
            std::list::const_iterator iter;
            for(iter = m_reports.begin();
                 iter != m_reports.end();
                 ++iter)
            {
                iter->report();
            }
        }

        private:
            std::list m_reports;
};

This way, the ReportManager doesn’t have to know any­thing about any Report.

Tags: ,

No responses yet

Leave a Reply