CODEWITHSUNDEEP
c++abstract-classvirtual-functions
Eric
Eric

Reputation: 1243

Are empty methods in an abstract class/interface considered a good practice?

Let's assume you have an base class for the different states of a State Machine that has methods for different inputs like mouse, keyboard, joystick, etc. Now not every derived state is going to use all possible types of inputs. If the base class methods are pure virtual every derived state class needs to always implement every single one of them. To avoid this i declared them with an empty body in the base class and just override the ones that are used by the particular derived class. In case the class doesn't use a certain input type the empty base class method get's called. I am storing the currentState in a base class pointer and just feed it with the input without having to know which particular derived state it actually is to avoid unnessecary casts.

class Base
{
  public:
    virtual void keyboardInput() {}
    virtual void mouseInput() {}
};

class Derived : public Base
{
  public:
    void keyboardInput()
    {
        // do something
    }

    // Derived doesnt use mouseInput so it doesn't implement it    
};

void foo(Base& base)
{
    base.keyboardInput();
    base.mouseInput();
}

int main()
{
    Derived der;
    foo(der);
}

Is this considered a good practice?

Upvotes: 1

Views: 977

Answers (3)

πάντα ῥεῖ
πάντα ῥεῖ

Reputation: 1

Your question is opinion based, but I'd rather follow this approach to use an interface:

struct IBase {
    virtual void keyboardInput() = 0;
    virtual void mouseInput() = 0;
    virtual ~IBase() {}
};

class Base : public IBase {
  public:
    virtual void keyboardInput() override {}
    virtual void mouseInput() override {}
};

class Derived : public Base {
  public:
    void keyboardInput() override {
        // do something
    }

    // Derived doesnt use mouseInput so it doesn't implement it    
};

int main() {
    std::unique_ptr<IBase> foo  = new Derived();

    foo->keyboardInput();
    foo->mouseInput();

    return 0;
}

Some arguments why that's better practice added from the comments:

  • The idea is that interface should contain as little assertions as possible, making it less likely to change, making it more dependable for those who inherit from it. Implementing the methods, albeit empty, is already an assertion, however small.
  • It would be less pain for refactorings coming later, which introduce more interfaces with multiple inheritance.

Upvotes: 4

Tom&#225;š Zato
Tom&#225;š Zato

Reputation: 53267

It really depends on what you want from the methods. When declaring an interface, usually the methods are left pure virtual because they are required to be implemented for the class to work at all. Marking them pure virtual signals "You have to implement this.".

However, sometimes there are methods that may do nothing and it's valid for all possible implementations for them to do nothing. It is not very common, but it is possible.

I don't think that your interface is the case though, and you should follow @πάντα ῥεῖ's answer. Or do it through multiple inheritance:

class MouseInput {
  public:
    virtual void mouseInput() = 0;
}

class KeyboardInput {
  public:
    virtual void keyboardInput() = 0;
}

class Derived : public KeyboardInput
{
  public:
    virtual void keyboardInput() override
    {
        // do something
    }
};

class AllInput : public KeyboardInput, public MouseInput
{
  public:
    virtual void keyboardInput() override
    {
        // do something
    }
    virtual void mouseInput() override
    {
        // do something
    }
};

That has the benefit that you can have methods that explicitly say that they work with one kind of input:

void doSomethingMouseIsh(MouseInput* input);

The disadvantage is that methods that combine mouse and keyboard input get weird unless you have InterfaceAllInput as interface and use it for all "all input methods"

Final note: as long as you try to write clean code, considering each use case is more important than some best practices.

Upvotes: 2

user11956505
user11956505

Reputation:

If you going to be strict about it this does violate ISP (https://en.wikipedia.org/wiki/Interface_segregation_principle) as your forcing a subclass to depend on a method it doesn't use - but generally its not too bad in practice if the alternative adds more complexity.

Upvotes: 1

Related Questions