1

I find myself often in the situation where I want to mock a non-copyable object, for example a DbHandle handle.
I was going back and forth looking at different design choices, and I settled on the following design:

In my production code, there is no need for a client of the Repository to know the DbHandle object, but for the unit tests, this DbHandle object is exactly the one I want to mock.
I settled for unique_ptr because they make clear that the Respository owns the DbHandle, even if it is created outside.

#include <memory>

class Repository {
public:
    Repository() : d_dbHandel(std::unique_ptr<DbHandel>(new DbHandle("default"))) {
        d_dbHandel->open("myDB");
    }

    Repository(std::unique_ptr<DbHandle> DbHandle) :
        d_dbHandel(std::move(DbHandle) {
    }

    inline DbHandle& getDbHandel() {
        return *d_dbHandel;
    }

private:
    std::unique_ptr<DbHandel> d_dbHandel;
};

What are some of the flaws of this approach?

Improve this question
6
  • 1
    I'm not sure what it is that you're trying to accomplish here. All I see is a lot of boilerplate for a DBHandle object (but no other functionality), which by your own admission is not even needed. Commented Jul 28, 2016 at 22:50
  • Thanks for your comment. What I try to accomplish is to be able to injest a mocked DBHandle object in the scenario where DBHandle is non-copiable. Commented Jul 28, 2016 at 23:14
  • 1
    Yes, but what is the purpose of that? What will it accomplish? Commented Jul 29, 2016 at 2:40
  • @Robert Harvey Mocking, for example a database connection, allows you to unit test the behavior of the class owning the db connection without actually having to establish any connection. While in production the db connection is entirely hidden inside the Repository during testing, one has to have the chance to set it. I want to achieve this without compromising my business logic (too much). Commented Jul 29, 2016 at 13:56
  • Usually, what you are mocking is already exposed as a touch point for the actual class you are testing. It is unusual to expose a touch point for testing that won't actually be used in production. That would mean you are testing code that isn't in the production system. Commented Jul 29, 2016 at 13:58

1 Answer 1

Reset to default
1

What you do is mostly correct.

You can ask yourself if you are not exposing your code simply because of testing. This is indeed a shortcoming. The client does not need to know about the DbHandle, but the Repository does not either! Strictly speaking, how can you unit-test the repository if it depends on (knows) a concrete implementation DbHandle?

Therefore the solution is to always inject DbHandle, either from the unit test or from a wrapper class or factory function and to use an abstract class DbHandle. Create a derived concrete class DbDefault that is normally used.

Also I do not recommend exposing members via getters as in your getDbHandel() function.

This could be an improved design:

#include <memory>

std::unique_ptr<Repository> createRepository()
{
    return std::make_unique<Repository>( std::make_unique<DbDefault>() );
} 

class Repository {
public:
    explicit Repository(std::unique_ptr<DbHandle> dbHandle) :
        d_dbHandel(std::move(dbHandle) {
}

void storeValue(double x) {
    d_dbHandel->storeValue(x);
}

private:
    std::unique_ptr<DbHandel> d_dbHandel;
};
Improve this answer

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.