8

In C++ I often use RAII-style objects to make code more reliable and allocate them on stack to make code more performant (and to avoid bad_alloc).

But creating an object of concrete class on stack violates the dependency inversion (DI) principle and prevents mocking this object.

Consider the following code:

struct IInputStream
{
    virtual vector<BYTE> read(size_t n) = 0;
};

class Connection : public IInputStream
{
public:
    Connection(string address);
    virtual vector<BYTE> read(size_t n) override;
};

struct IBar
{
    virtual void process(IInputStream& stream) = 0;
};

void Some::foo(string address, IBar& bar)
{
    onBeforeConnectionCreated();
    {
        Connection conn(address);
        onConnectionCreated();
        bar.process(conn);
    }
    onConnectionClosed();
}

I can test IBar::process, but I also want to test Some::foo, without creating real Connection object.

Surely I can use a factory, but it will significantly complicate code and introduce heap-allocation.
Also, I don't like to add the Connection::open method, I prefer to construct completely initialized and fully functional objects.

I would make Connection type a template parameter for Some (or for foo if extract it as a free function), but I'm not sure that it's right way (templates look like a black magic to many people, so I prefer to use dynamic polymorphism)

BenMorel
  • 34,448
  • 50
  • 182
  • 322
Abyx
  • 12,345
  • 5
  • 44
  • 76
  • 2
    Templates shouldn't be black magic to more or less competent C++ programmer, I see no reason to avoid them. Also I don't think heap allocation is *that* expensive (this, of course, depends on the software you write), so I see no reason to avoid it either (when used with smart pointers). – Alex B Oct 18 '11 at 12:15
  • 5
    @Alex B: there sort of is a reason to avoid them, although I agree that it's not because they're "black magic". It's because if everything is injected via template parameters, then everything you write is a template, your library is header-only, and that can get quite cumbersome in terms of either compilation or distribution. Although, I suppose that with care you could unit test the header-only library, then build from it a TU that only contains the instantiations that the application needs. – Steve Jessop Oct 18 '11 at 12:23
  • 1
    RAII and DI work great together, so the title is misleading, your issue is Stack Allocation vs DI. – Matthieu M. Oct 18 '11 at 12:27
  • @Steve Jessop: most of my code is header-only, because it's easier to #include header rather than link .lib compiled with right compiler settings (easier than autolinking and -s/-gds/etc suffixes) – Abyx Oct 18 '11 at 13:22
  • @Abyx: as a side note: add virtual destructors to your interfaces or objects using them will not be able to destroy the actual objects implementing that interface. – jupp0r Oct 28 '15 at 12:19

3 Answers3

7

What you are doing right now is "force-coupling" the RAII class and the service provider class (which, if you want testability, should really be an interface instead). Address this by:

  1. abstracting Connection into IConnection
  2. have a separate ScopedConnection class that provides RAII on top of that

For example:

void Some::foo(string address, IBar& bar)
{
    onBeforeConnectionCreated();
    {
        ScopedConnection conn(this->pFactory->getConnection());
        onConnectionCreated();
        bar.process(conn);
    }
    onConnectionClosed();
}
Jon
  • 428,835
  • 81
  • 738
  • 806
  • 2
    And accept that `ScopedConnection` doesn't need to be mocked, it's "safe" to use the real version even in tests that are supposed to isolate `Some::foo`. Or if that's unacceptable, grit your teeth and inject it as a template parameter, or use `scoped_ptr` to provide the RAII, which as a standard class (or 3rd party if you're still on C++03) is an acceptable hard dependency. – Steve Jessop Oct 18 '11 at 12:16
  • That's what I wrote about factory. In order to follow your answer, I should create a factory just for Connection, or factory for a lot of unrelated classes (as you suggest). Bring this factory to `Some` through a lot of layers (or make it global). – Abyx Oct 18 '11 at 12:53
  • @Abyx: The factory would be a prime candidate for DI, which would be preferable to passing it through manually or having a global. But you need it to increase abstraction. – Jon Oct 18 '11 at 12:57
2

To chose between your actual implementation and the mocked one, you have to inject the actual type that you want to construct in some fashion. The way I'd recommend is injecting the type as an optional template parameter. It allows you to unobtrusively use Some::foo as you used to, but enables you to swap the created connection in case of a test.

template<typename ConnectionT=Connection> // models InputStream
void Some::foo(string address, IBar& bar)
{
    onBeforeConnectionCreated();
    {
        ConnectionT conn(address);
        onConnectionCreated();
        bar.process(conn);
    }
    onConnectionClosed();
}

I would not create the overhead of a factory and runtime polymorphism if you know the actual type at compile time.

jupp0r
  • 4,502
  • 1
  • 27
  • 34
2

By "I can use a factory, but it will significally complicate code and introduce heap-allocation" I meant the following steps:

Create abstract class and derive Connection from it

struct AConnection : IInputStream
{
    virtual ~AConnection() {}
};

Add factory method to Some

class Some
{
.....
protected:
    VIRTUAL_UNDER_TEST AConnection* createConnection(string address);
};

Replace stack-allocated connecton by smart pointer

unique_ptr<AConnection> conn(createConnection(address));
Abyx
  • 12,345
  • 5
  • 44
  • 76