views:

57

answers:

3

I'm trying to wrap my head around implementing MVC with as much useful abstraction as I can for purposes of automated unit testing. In the process, I came up against an interesting conundrum: How and where do I declare the mock object for my database?

Here's what I have.

  • ContactView is a form that implements IContactView. It has knowledge of IContactModel so that it can update itself in response to event notifications from the model object.
  • Contact is a class that implments IContactModel. It encapsulates the business rules for manipulating the object, as well as the code for fetching from/updating data in the data access layer.
  • ContactController is a class that implements IContactController and has knowledge of both IContactModel and IContactView.
  • Database is a class that implements IDatabase and contains methods for selecting, inserting, updating, and deleting data in the database.

This, to me, is the tricky part. The model object needs to know how to interact with the IDatabase so that it can operate on either a real database or a mock object. The problem I am faced with is how to provide a reference to the database without violating Separation of Concerns.

I'd rather that the views and controllers had no knowledge of how the data was stored. That way, if I chose to do so later, I could swap out IDatabase for something like IJsonStore or IXmlStore and only have to touch the model classes. I don't want my views & controllers making any assumptions about where and how the data is stored.

I see a couple of probable solutions, but I am not certain what the best one is.

  • I can declare a singleton that exposes a public property, Database (of type IDatabase). This way, the unit tests could set the database to a mock object, and production code would set it to a production database. But that means that production code, at some point, would have to know about IDatabase. I suppose that's unavoidable; but I am hoping there's some other solution that's preferable.
  • I can modify the model classes to maintain a reference to the database, taking it in the constructor. This seems undesirable simply because it results in a lot of additional code. And, I'm brought right back to square one: Whomever declares the model instance has to know which IDatabase object I want to use.

I'm sure there are alternatives out there, and that I'm just unaware of them. So I'm throwing this out there: How would you guys do this, and what have you seen that worked well?

Thanks in advance.

+1  A: 

It is often better to keep Model objects as POCOs/POJOs, and have the Controllers populate Model (and View) using injected dependencies.

For lots of different reasons, Constructor Injection is your best default choice for DI. Here's a C#-based example of a Controller with an injected IDatabase:

public class ContactController : IContactController
{
    private readonly IDatabase db;

    public ContactController(IDatabase db)
    {
        if (db == null)
        {
            throw new ArgumentNullException("db");
        }

        this.db = db;
    }

    public IContactView CreateView(int id)
    {
        var model = this.db.Select(id);
        return new ContactView(model);
    }
}

I don't know whether this looks like your existing interfaces, but it should be enough to give you an idea. Notice how the readonly keyword and the Guard Clause collaborate to make the injected dependency an invariant of the ContactController so that the rest of code is guaranteed that it's always present.

You can use either Poor Man's DI or a proper DI Container to wire your application up in the application's entry point. This would be where you map IDatabase to a concrete implementation, allowing you to adhere to the Liskov Substitution Principle in the rest of the code.

Mark Seemann
+2  A: 

As you pointed out, you have to connect persistence to the Model. If you think this is a dependency (the Model just can't function without it), then pass it into the constructor. That tells the world that a Model must have access to persistence and not to create a Model until you have everything it needs. A constructor doesn't seem like a lot more code than a property, and it's safer.

Second, I wouldn't call that thing an IDatabase. The word Database describes the implementation, not the role. Depending on your domain, I might call it an AddressBook, or Rolodex, or something that tells me about the context of the application. This keeps the model code free of any technical implementation details. If I were using a database to implement it, I might then call the class DatabaseAddressBook. I usually find that being picky about this sort of separation produces clearer code and (sometimes) clearer thinking.

Steve Freeman
Excellent description, I agree completely. Abstraction is very important to making the code clearer and keep separation of concerns ... just be sure to not overthink it and have too much abstraction b/c then nobody can tell what the thin is actually doing.
Brian T Hannan
A: 

I solve this kind of problem the simplest possible way: by making the Database class a static facade (which has no need to implement a separate Java interface).

This solution not only makes the production code as simple as it can be, but also the unit tests! For example, here is an almost complete JUnit test, using the JMockit Expectations API (a tool I created to enable such tests):

public final class Database
{
    private Database() {}

    public static void save(Object transientEntity) { ...uses JPA/JDO... }
    ... other static methods ...
}

public class Contact
{
    public Contact(String s, int i) { ... }

    public void doSomeBusinessOperation()
    {
        ...
        Database.save(<some transient entity>);
        ...
    }
}

public final class ContactTest
{
    @Test
    public void doSomeBusinessOperationShouldSaveEntityToDatabase()
    {
        new Expectations()
        {
            Database db; // a mock field, causing "Database" to be mocked

            {
                // Records an expectation for the method to be called once:
                Database.save(any);
            }
        };

        // Exercises SUT, which should replay the expectation as recorded:
        new Contact("abc", 123).doSomeBusinessOperation();
    }
}
Rogerio