sjrowlinson
sjrowlinson

Reputation: 3355

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.

Upvotes: 4

Views: 184

Answers (2)

Disillusioned
Disillusioned

Reputation: 14832

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.

Upvotes: 2

Walter
Walter

Reputation: 45444

Your fundamental problem here is the inconsistency of the interface declared by the abstract base class. Is this supposed to be a 2D or 3D interface? The way you declared it, it's both, but the derived classes are either and hence do not fully implement the interface: this causes all your problems.

Think about how are you going to use the base class, perhaps you can make it a template over the number of dimensions? If there are more virtual methods inpendent of the dimensionality, they could be in a base, e.g.

struct DLAContainerBase
{
  /* some basic virtual interface */
};

template<int Dims>
struct DLAContainer : DLAContainerBase
{
  virtual void spawn_particle(std::array<int,Dims>&,
                              std::uniform_real_distribution<>&) = 0;
};

But w/o knowing how the base is going to be used, I cannot really give you reliable advise. Btw, you can also provide an implementation for a pure virtual method (e.g. one that throws).

Upvotes: 2

Related Questions