tags:

views:

75

answers:

4

Suppose I have a C++ application for a pet store that started out just with Cats. The app has a class that does some data handling for a Cat dialog box:

CatDlg.cpp:

CatDlg::CatDlg() {//ctor stuff}

CatDlg::onInitDialog() {
   DBAccess db;
   db.open();
   CatRec cat;
   cat = db.findRec(m_catID);
   CString name = cat.getName();

   NameField.SetWindowText(name);
   // other catty dialogy stuff
}

Now the store owner wants to add Dogs to the application. Dog data is essentially the same as Cat data, except for data types. I could just duplicate the CatDlg class, change a few types and handle it this way, but that leads to maintenance issues and code bloat with 99% duplicated code:

DogDlg.cpp:

DogDlg::onInitDialog() {
   DBAccess db;
   db.open();
   DogRec dog;
   dog = db.findRec(m_dogID);
   CString name = dog.getName();

   NameField.SetWindowText(name);
   // other doggy dialogy stuff
}

Or, I could change the dialog class to handle multiple animals, by passing in a flag that indicates the type of animal, then process it accordingly. THis also gets uglier as the owner add new animals, because the method gets longer, with lots of duplicated code blocks:

AnimalDlg.cpp:

AnimalDlg::onInitDialog(AnimalType type) {
    DBAccess db;
    db.open();
    if(type == CAT)
    {
        CatRec cat;
        cat = db.findRec(m_catID);
        CString name = cat.getName();
    }
    else if(type == DOG)
    {
        DogRec dog;
        dog = db.findRec(m_dogID);
        CString name = dog.getName();
    }

   NameField.SetWindowText(name);
}

What's the best way to refactor the class to make it more general? Is there a design pattern that I can apply? Is there some way to pull the common code into a base class, then make animal-specific subclasses? I had a quick look through Fowler's book on Refactoring, but nothing leapt out at me.

Assume there is lots more code in the class, at the moment specific to the Cat type, but applicable to any type of animal.

Thanks for your help!

Philip

+5  A: 

This screams for inheritance. Define a base class for animal. Then derive cat and dog and striped beaver and whatever else the store wants to sell.

Amardeep
+2  A: 

Assuming that CatRec and DogRec are unrelated types that can't be made part of an inheritance structure, I think you need to use templates.

AnimalDlg would be a parent of CatDlg and DogDlg. AnimalDlg would have a new protected template findRec function whose return type would be the template type (so you could return CatRec or DogRec). The onInitDialog would exist only in AnimalDlg and call a virtual function getName instead of the db-specific code. The child versions of getName would then use the appropriate version (for example findRec<CatDlg>) of findRec to get the record and return the type back to onInitDialog.

I'll try to sketch the code later if I have a chance.

EDIT (code sketch):

class AnimalDlg
{
public:
    void onInitDialog()
    {
        NameField.SetWindowText(getName());
    }

protected:
    template <class T>
    T findRec(IdType id) const
    {
        DBAccess db;
        db.open();
        return T(db.findRec(id));
    }

    virtual std::string getName() const = 0;
};

class CatDlg : public AnimalDlg
{
protected:
    virtual std::string getName() const
    {
        return findRec<CatRec>(m_catID).getName();
    }
};
Mark B
+1  A: 

Tying your DB and data so directly to the GUI is not so good. If all you want is CRUD (Create Read Update Delete) then there are many tools available to do that with a DB already.

Drive the GUI around the purpose of your app not how it stores the data.

Greg Domjan
That's a good point - while this example is a simplification of the actual problem, it probably does abuse MFC's MVC architecture...
phipster
A: 

I would go along the lines of template specialization:

template <class AnimalType>
AnimalDlg::onInitDialog(AnimalType type, int m_animalId) {
   DBAccess db;
   db.open();

   name = db.findRec<AnimalType>(m_animalId).getName();

   NameField.SetWindowText(name);
}
Edison Gustavo Muenz