When do I use an interface on a DTO model?

Question

I'm using the Moq framework for my unit tests, and I recently wrote a test for checking that a property on one of my DTOs was set within a method.

Originally, the DTO was returned by my methods....then the property set. Here's the method that I'm testing.

public void UpdatePlayerProfileStatistics(DataRow playerRecord, bool useUpdatedGamesPlayed = true)
    {
        DTOProfile.Player player = this._playerDataParser.ParsePlayerProfileData(playerRecord);

        DTOProfile.Player playerInDatabase = this._playerManager.MatchPlayerInDatabase(player.Id);

        string errorMessage = String.Empty;
        if (useUpdatedGamesPlayed)
        {
            // do nothing. player.Games = player.Games; nothing changes
        }
        else
        {
            // we want to keep the Games played from the value of the player in the database.
            player.Games = playerInDatabase.Games;
        }

        if (!this.repository.UpdatePlayerProfile(player, out errorMessage))
        {
            throw new InvalidOperationException(errorMessage);
        }
    }

For my test, I would mock ParsePlayerProfileData and MatchPlayerInDatabase, but to test the setting of the .Games property, I needed to add an IPlayer interface to Player. I'd then make ParsePlayerProfileData and MatchPlayerInDatabase to return the interface.

DTOProfile.IPlayer player = this._playerDataParser.ParsePlayerProfileData(playerRecord);

DTOProfile.IPlayer playerInDatabase = this._playerManager.MatchPlayerInDatabase(player.Id);

Then in my test, I have:-

var mockUpdatedPlayer = new Mock<IPlayer>();
var mockDatabasePlayer = new Mock<IPlayer>();
_playerDataParser.Setup(p => p.ParsePlayerProfileData(It.IsAny<DataRow>())).Returns(mockUpdatedPlayer.Object);
_playerManager.Setup(m => m.MatchPlayerInDatabase(It.IsAny<int>())).Returns(mockDatabasePlayer.Object);

UpdatePlayerProfileStatistics(myRow, true);

mockedPlayer.VerifySet(x => x.Games = It.IsAny<int>(), Times.Never());

This all works.

My question is, moving forward should I have interfaces for all my DTOs, and make all my methods return these interfaces (instead of the concrete type)? I figure that this will allow me perform these 'VerifySet' and 'VerifyGet's.

Or, is there a better way to test this?

score 2 · Answer 1 · answered Jul 06 '14 at 19:20

You don't want to mock the DTO, the correctly-constructed DTO is the result of your testing, and thus the thing you want to be asserting against.

The method should return an instance of the DTO in question, which you can then validate. No interface necessary.

If you mock every aspect of the interaction, all you're testing is that the mocking framework works.

My gut instinct is that your method here is doing too much... A suggestion for refactoring:

public DTOProfile.Player UpdatePlayerProfileStatistics(DataRow playerRecord, bool useUpdatedGamesPlayed = true)
{
    DTOProfile.Player player = this._playerDataParser.ParsePlayerProfileData(playerRecord);

    DTOProfile.Player playerInDatabase = this._playerManager.MatchPlayerInDatabase(player.Id);

    string errorMessage = String.Empty;
    if (useUpdatedGamesPlayed)
    {
        // do nothing. player.Games = player.Games; nothing changes
    }
    else
    {
        // we want to keep the Games played from the value of the player in the database.
        player.Games = playerInDatabase.Games;
    }

    return player;
}

public void PersistPlayer(DTOProfile.Player player) 
{
    if (!this.repository.UpdatePlayerProfile(player, out errorMessage))
    {
        throw new InvalidOperationException(errorMessage);
    }
}

You can then test both methods:

Given a DataRow, the player object is updated properly.
Given a Player instance, an exception is thrown if persisting fails, the correct error message is generated under various circumstances, etc.

score 1 · Answer 2 · edited May 23 '17 at 10:32

I think modifying your classes with code used only for testing is a design smell. It will clutter your DTOs and will lead to code duplication. Besides, all interfaces will basically repeat all properties found in DTOs, and to add/remove/modify property you will now need to change two places instead of one.

You can rewrite your unit tests without mocking DTOs:

var mockUpdatedPlayer = new DTOProfile.Player();
var mockDatabasePlayer = new DTOProfile.Player();
_playerDataParser.Setup(p => p.ParsePlayerProfileData(It.IsAny<DataRow>())).Returns(mockUpdatedPlayer);
_playerManager.Setup(m => m.MatchPlayerInDatabase(It.IsAny<int>())).Returns(mockDatabasePlayer);
UpdatePlayerProfileStatistics(myRow, true);
Assert.AreNotEqual(mockUpdatedPlayer.Games, mockDatabasePlayer.Games);

Or, if there is no empty player constructor, or construction is too heavy for tests, you can mark your DTOs properties as virtual - Moq can mock virtual properties.

Several more links supporting my point of view:

http://rrees.me/2009/01/31/programming-to-interfaces-anti-pattern/

http://marekdec.wordpress.com/2011/12/06/explicit-interface-per-class-antipattern/

For a completely opposite point of view:

Are single implementer interfaces for unit testing an antipattern?

Yeah, the design smell is exactly what I was referring to. I like the idea of having the VerifySet and VerifyGet methods....but having to convert huge chunks of my code base in order to use them really doesn't please me. I like the idea of the virtual properties. I'll try them out and see if that alleviates some of my concerns. — Sean Holmesby, Jul 07 '14 at 00:14

When do I use an interface on a DTO model?

2 Answers2