Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Code refactoring with existing classes in C++

I code in C++ and since the interface keyword is not there in the language, I will use interface as a concept( a contract) here.

Suppose you have an interface say IBase, which has been implemented by dozens of classes. Now you need to add another method in that interface IBase.

What would be the approach to make that change in minimal way to solve the problem of overriding that method in all implementing classes?

One way could be adding the new method with a default implementation in the Ibase class and have the derived class that need it override it.

In the above solution I feel like I am going wrong at two places, breaking the open closed principle by touching the interface and also breaking the contract by telling the user you can also call another method.

Another thing that strikes me here is , I am adding a method to the base and only one derived class is overriding it , which basically means that the other classes don’t need that method.

So that brings me to the another solution, where the class which needs this method would inherit from another class which provides this feature or use composition to use the feature of another class.

With the above solution I hit another problem, how would the client access the new functionality of the derived class through the Ibase interface. Dynamic dispatch in C++ would only work if the function is present in the base class.

Can someone please help!

UPDATE

I just coded up my understanding based on the comments below, but now the client code looks messy, it have to know which interface to use for extra functionality. Should we abstract the code below using factory/ abstract factory ?

#include <iostream>

using namespace std;


class IBase {
    public:
     virtual void test() = 0; 
};

class INewInterface {
    public:
     virtual void newMethod() = 0; 
};

class Derived : public IBase {
    public:
    virtual void test () {
        cout<< " Derived  " << endl;
    }
};

class DerivedSecond: public IBase, public INewInterface {
    public:
    virtual void test() {
        cout<< " Derived Second " << endl;
    }

    void newMethod( ) {
        cout<< " newMethod " << endl;
    }
};

int main( int argc, char ** argv ) {

    // Client code
    //Client needs to cast to two different interfaces to get access to the new functionality.
    // Probably should use a factory here 
    INewInterface * pNewInterfacePtr = dynamic_cast< INewInterface * > ( new DerivedSecond );
    IBase *         pIbasePtr        = dynamic_cast< IBase * > ( new DerivedSecond );

    pIbasePtr->test();
    pNewInterfacePtr->newMethod();
    return 0;
}
like image 644
deb Avatar asked Sep 27 '22 00:09

deb


2 Answers

It is difficult to really know what will suit your situation because you evidently have a working system which may or may not benefit from anything we might suggest.

However I have produced an example of how you can manage multiple subtypes with separate and/or overlapping responsibilities. This approach is to have a master container holding ownership of all the objects using a std::unique_ptr to ensure they all get deleted and we have no memory leaks.

In addition to that master container we have separate containers for different views. Each view holds references (raw pointers) to those elements with a specific responsibility not shared with other types.

Perhaps it may be of some use in your situation, perhaps not:

#include <vector>
#include <memory>
#include <iostream>

class Role
{
public:
    virtual ~Role() {}
};

class RoleOne
: public virtual Role
{
public:
    virtual void do_role_one_stuff() = 0;
};

class RoleTwo
: public virtual Role
{
public:
    virtual void do_role_two_stuff() = 0;
};

class ItemA
: public RoleOne
{
public:
    void do_role_one_stuff() override { std::cout << "Item A in Role One\n"; }
};

class ItemB
: public RoleOne
, public RoleTwo
{
public:
    void do_role_one_stuff() override { std::cout << "Item B in Role One\n"; }
    void do_role_two_stuff() override { std::cout << "Item B in Role Two\n"; }
};

class ItemC
: public RoleTwo
{
public:
    void do_role_two_stuff() override { std::cout << "Item C in Role Two\n"; }
};

class Resources
{
    // unique_ptr ensures deletion (no memory leaks)
    std::vector<std::unique_ptr<Role>> all; // owning container

    // raw 'access' pointers share access (no need to share ownership)
    std::vector<RoleOne*> ones; // alternate 'view' (no ownership)
    std::vector<RoleTwo*> twos; // alternate 'view' (no ownership)

public:
    void add_item(Role* item)
    {
        // manage ALL items life-spans here
        all.emplace_back(item);

        // add one-centric items to the one-centric view
        if(auto one = dynamic_cast<RoleOne*>(item))
            ones.emplace_back(one);

        // add two-centric items to the two-centric view
        if(auto two = dynamic_cast<RoleTwo*>(item))
            twos.emplace_back(two);
    }

    void do_business()
    {
        // ItemA and ItemB types do this kind of business
        std::cout << "\nDoing role one business:\n";
        for(auto role: ones)
            role->do_role_one_stuff();

        // ItemB and ItemC types do this kind of business
        std::cout << "\nDoing role two business:\n";
        for(auto role: twos)
            role->do_role_two_stuff();
    }
};

int main()
{
    Resources res;

    res.add_item(new ItemA);
    res.add_item(new ItemB);
    res.add_item(new ItemC);
    res.add_item(new ItemB);
    res.add_item(new ItemA);
    res.add_item(new ItemC);

    res.do_business();
}

Output:

Doing role one business:
Item A in Role One
Item B in Role One
Item B in Role One
Item A in Role One

Doing role two business:
Item B in Role Two
Item C in Role Two
Item B in Role Two
Item C in Role Two
like image 139
Galik Avatar answered Oct 06 '22 01:10

Galik


Remember, principles are not rules. You will find sometimes that you have to break some principle in the real worl and it will be a good decision if you understand what you are doing. Then, the option of creating a new dummy method in IBase could be your solution.

If you do not want to break the actual interface, I would use composition and delegation. But I do not know your issue. Maybe my solution does not fit in your case.

class IBase {
public:
    virtual void test() = 0;
};
class INewIfc {
protected:
    IBase* ifc;
public:
    INewIfc(IBase* _ifc) : ifc(_ifc) {}
    ~INewIfc() {}
    virtual void newMethod() = 0;
    IBase* getIfc() {return ifc;}
}

Then you create your concrete classes from IBase (what you had) or use composition of IBase and the new interface to create new ones without affecting the previous classes.

like image 33
blashser Avatar answered Oct 06 '22 00:10

blashser