4

In the process of refactoring non-testable code we are re-designing some of ours classes so that it can be easily replaced by a stub or mock during unit tests.

Here is an example of such class (implementation is omitted for simplification):

class LightController {
public:
   void turnOn();
private:
    CanBus m_canBus;
};

The turnOn() method uses internally the CanBus which itself deals with hardware communication (implementation is omitted for simplification):

class CanBus {
public:
    void write(std::array<unsigned char, 8> data);
};

We can't test LightController as is because CanBus won't work without proper hardware environment. The refactoring is planned as follow:

  • Pass The CanBus by dependency injection to the LightController
  • Create a stub class with no hardware dependency to replace CanBus during tests

About the second steps, we can either make functions virtual and create a derived class:

class CanBus {
public:
    virtual void write(std::array<unsigned char, 8> data);
};

class CanBusStub : public CanBus {
public:
    void write(std::array<unsigned char, 8> data) override;
};

Alternatively, we can suggest the creation of a proper interface class and make both CanBus and CanBusStub derive from it:

class CanBusInterface {
public:
    virtual void write(std::array<unsigned char, 8> data) = 0;
};

class CanBus : public CanBusInterface {
public:
    void write(std::array<unsigned char, 8> data) override;
};

class CanBusStub : public CanBusInterface {
public:
    void write(std::array<unsigned char, 8> data) override;
};

I know this is probably an opinion-based question which is extremely dependent on the context, but given our team is fairly new with unit tests and that none of our codebase is based on Interface, would you suggest to go the simpler way (just makes CanBus methods virtual) or the complete way (enforcing creation of interfaces)?

Is there any important downside with making the CanBus methods virtual?

Delgan
  • 366
  • 2
  • 13
  • 2
    Marking the function `virtual` suggests to the next person reading your code that these function can (or should) be overloaded in a derived class; using interfaces suggests your intent more clearly. – Robert Harvey Aug 25 '21 at 16:03

3 Answers3

5

If you have to use virtual functions to stub things out, write your pure virtual interface and make the implementation final, not override. That way you aren't tempted to subclass CanBus and break it's invariants.

Is there any important downside with making the CanBus methods virtual?

They can't be template member functions.

But in C++ you have another option, which doesn't add virtual anywhere.

class CanBus {
public:
    void write(std::array<unsigned char, 8> data); // does real write
};

template <typename Bus>
class BasicLightController {
public:
   void turnOn(); // calls m_CanBus.write as before
private:
    Bus m_canBus;
};

Your integrated code can still have a concrete type

using LightController = BasicLightController<CanBus>;

And test code can define it's own things

class CanBusStub {
public:
    void write(std::array<unsigned char, 8> data); // whatever the test needs
};

optionally, requires C++20

template <typename T> concept CanBus = 
requires(T bus, std::array<unsigned char, 8> data) {
    bus.write(data); 
}

template <CanBus Bus>
class BasicLightController { ... };
Caleth
  • 10,519
  • 2
  • 23
  • 35
  • Thanks for your suggestion. I often thought about using template but doesn't it prevent IDE to provide useful completion? The `Bus` typename being generic, I refrained myself from using it as it hides useful information compared to polymorphism. – Delgan Aug 26 '21 at 07:36
  • @Delgan This absolutely *is* polymorphism. It's just not *runtime* polymorphism. I don't know what the state of the art is w.r.t. IDEs providing autocompletion on concepts, but it is certainly possible – Caleth Aug 26 '21 at 07:46
  • Thanks. Interesting. But what if the template class should be used as a class member, or inside a function ? How to make sure the code works both in prod or testing env ? – Zyend Nov 30 '22 at 10:59
2

The virtual method option is pretty disruptive to your design. It is a big change just to do a test. And you would be testing something different from what would be used in production.

The interface option is cleaner. Your class would use the same CanBus interface type both in production and while running the test. No extra plumbing to support the test scenario. So this is the common and preferred way to do it.

Martin Maat
  • 18,218
  • 3
  • 30
  • 57
1

Actually, you are about to replace almost all implementations of the current class. That is, the mock is a completely different class with different behavior, just the signatures of methods/properties are shared.

That's not the situation a programmer dealing with your classes would expect: a derived class normally uses a lot of the implementations offered by the base class.

So, I suggest to use the interface approach.

Bernhard Hiller
  • 1,953
  • 1
  • 12
  • 17