How can I avoid adding getters to facilitate unit testing?

Question

After reading a blog post mentioning how it seems wrong to expose a public getter just to facilitate testing, I couldn't find any concrete examples of better practices.

Suppose we have this simple example:

public class ProductNameList
{
    private IList<string> _products;
    public void AddProductName(string productName)
    {
        _products.Add(productName);
    }
}

Let's say for object-oriented design reasons, I have no need to publicly expose the list of products. How then can I test whether AddProductName() did its job? (Maybe it added the product twice, or not at all.)

One solution is to expose a read-only version of _products where I can test whether it has only one product name -- the one I passed to AddProductName().

The blog post I mentioned earlier says it's more about interaction (i.e., did the product name get added) rather than state. However, state is exactly what I'm checking. I already know AddProductName() has been called -- I want to test whether the object's state is correct once that method has done its work.

Disclaimer: Although this question is similar to Balancing Design Principles: Unit Testing, (1) the language is different (C# instead of Java), (2) this question has sample code, and (3) I don't feel the question was adequately answered (i.e., code would have helped demonstrate the concept).

Answer 1

Unit tests should test the public API.

If you have "no need to publicly expose the list of products", then why would you care whether AddProductName did its job? What possible difference would it make if the list is entirely private and never, ever affected anything else?

Find out what affect AddProductName has on the state that can be detected using the API, and test that.

Very similar question here: Domain Model (Exposing Public Properties)

Answer 2

You could make your accessors protected so you can mock or you could use internal so that you can access the property in a test but that IMO would be wrong as you have suggested.

I think sometimes we get so caught up in wanting to make sure that every little thing in our code is tested. Sometime we need to take a step back and ask why are we storing this value, and what is its purpose? Then instead of testing that the value gets set we can then start testing that the behaviour of the component is correct.

EDIT

One thing you can do in your scenario is to have a bastard constructor where you inject an IList and then you test that you have added a product:

public class ProductNameList
{
    private IList<string> _products;

    internal ProductNameList(IList<string> products)
    {
        _products = products;
    }
    ...
}

You would then test it like this:

[Test]
public void FooTest()
{
    var productList = new List<string>();

    var productNameList = new ProductNameList(productList);

    productNameList.AddProductName("Foo");

    Assert.IsTrue(productList[0] == "Foo");
}

You will need to remember to make internals visable to your test assembly.

Answer 3

Make _products protected instead of private. In your mock, you can add an accessor.

Answer 4

To test if AddProductName() did it's job, instead of using a public getter for _ProductNames, make a call to GetProductNames() - or the equivalent that's defined in your API. Such a function may not necessarily be in the same class.

Now, if your API does not expose any way to get information about product names, then AddProductName() has no observable side effects (In which case, it is a meaningless function).

If AddProductName() does have side effects, but they are indirect - say, a method in ProductList that writes the list of product names to a file, then ProductList should be split into two classes - one that manages the list, and the other that calls it's Add and Get API, and performs side effects.