Search code examples
c++inheritancecompositevirtual-functions

Inheritance class design with "irrelevant" methods


Say I have the following abstract base class:

class DLAContainer {
public:
    DLAContainer() { std::random_device rd; mt_eng = std::mt19937(rd()); }
    virtual void generate(std::size_t _n) = 0;
protected:
    std::mt19937 mt_eng;

    virtual void spawn_particle(int& _x, int& _y,
        std::uniform_real_distribution<>& _dist) = 0;
    virtual void spawn_particle(int& _x, int& _y, int& _z,
        std::uniform_real_distribution<>& _dist) = 0;

    // ... among other methods to be overridden...
};

and two classes which inherit from DLAContainer:

class DLA_2d : public DLAContainer {
public:
    DLA_2d() : DLAContainer() { // initialise stuff }
    void generate(std::size_t _n) { // do stuff }
private:;
    std::queue<std::pair<int,int>> batch_queue;
    // ...

    void spawn_particle(int& _x, int& _y, 
        std::uniform_real_distribution<>& _dist) { // do stuff }
    void spawn_particle(int& _x, int& _y, int& _z,
        std::uniform_real_distribution<>& _dist) { // do nothing }

    //...
};

and

class DLA_3d : public DLAContainer {
public:
    DLA_3d() : DLAContainer() { // initialise stuff }
    void generate(std::size_t _n) { // do stuff }
private:;
    std::queue<std::tuple<int,int,int>> batch_queue;
    // ...

    void spawn_particle(int& _x, int& _y, 
        std::uniform_real_distribution<>& _dist) { // do nothing }
    void spawn_particle(int& _x, int& _y, int& _z,
        std::uniform_real_distribution<>& _dist) { // do stuff }

    //...
};

As you can see, there are two overloads of spawn_particle - one for a 2D lattice and the other for 3D, however both are pure virtual functions and so must be overridden/implemented in both DLA_2d and DLA_3d sub-classes where the 3D method will do nothing in DLA_2d and vice-versa for DLA_3d.

Of course, this works and everything functions normally but I can't help but feel that the design is a bit clumsy when having to implement irrelevant methods in each class.

Is there a better design pattern for this such as implementing separate interfaces (i.e. ISpawnParticle_2d and ISpawnParticle_3d) for the two derived classes? Or would composition rather than inheritance be favoured in such a scenario?

Edit: I should add that DLAContainer has several other methods (and fields). Some of these methods are defined (such that they can be used by both DLA_2d and DLA_3d) and others are pure-virtual similar to spawn_particle - this is why I have DLAContainer as an abstract base class in this case.


Solution

  • You're right, it is clumsy.
    And it's the result of a common mistake in OO design: Using inheritance merely to avoid code duplication when the subtype cannot be said to be IS A parent type.

    Currently you're able to call:

    DLA_3d d3;
    d3.spawn_particle(...) //The 2D version
    //and
    DLA_2d d2;
    d2.spawn_particle(...) //The 3D version
    

    With seemingly no "ill effects" by ignoring the call and doing nothing. The problem is that code calling spawn_particle needs to be aware that:

    • Calling the method might do nothing.
    • Or pre-check the type to know which method to call.

    Both of these impose unnecessary extra knowledge/work on the caller. And effectively make it more error-prone to use.

    PS: Note that throwing an error at runtime doesn't really fix the design. Because callers are now left with: "Calling the method might throw OR pre-check the type..."


    There are a number of ways you can improve your design. But ultimately you know what you're trying to achieve and will have to make that decision yourself.
    Here are a few ideas:

    • First a foremost consider the following design principle: Favour composition over inheritance.
      • You don't need to inherit to reuse code: you can contain/reference an instance of another object, and offload work by calling the contained/referenced object.
      • You mentioned that DLAContainer has a number of other fields and methods. How many of those can be moved to a different or multiple classes?
    • Does it really make sense for the container to be spawning particles? The container's responsibility should be to hold things. Are you following the Single Responsibility Principle? (I doubt it.)
    • Consider moving each spawn_particle method to the appropriate subclass. (Though I suspect this will leave you with very much the same kind of problems.)
    • Develop an abstraction for "particle". Then both "particle spawners" can have the same signature, but spawn different concrete instances of "particle" I.e. a 2D particle or a 3D particle.