Stack allocated RAII objects vs DI principle
Asked Answered
A

3

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)

Aguiar answered 18/10, 2011 at 12:10 Comment(5)
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).Pentastich
@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.Meat
RAII and DI work great together, so the title is misleading, your issue is Stack Allocation vs DI.Nnw
@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)Aguiar
@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.Grouty
J
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();
}
Jigger answered 18/10, 2011 at 12:12 Comment(3)
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.Meat
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).Aguiar
@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.Jigger
A
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));
Aguiar answered 18/10, 2011 at 13:3 Comment(0)
G
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.

Grouty answered 28/10, 2015 at 12:34 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.