views:

433

answers:

5

Say you have this shell of a class:

public class Number
{
    private int value;

    public Number()
        : this(0) {}

    public Number(int initialValue)
        : this(initialValue, 0, 100) {}

    public Number(int initialValue, int minimumValue, int maximumValue)
    {
        if (minimumValue > maximumValue)
            throw new ArgumentException("Minimum cannot be greater than maximum", "minimumValue");

        MinValue = minimumValue;
        MaxValue = maximumValue;
        Value = initialValue;
    }

    public int MinValue { get; private set; }
    public int MaxValue { get; private set; }

    public int Value
    {
        get { return value; }
        set
        {
            if (value < MinValue)
                value = MinValue;
            if (value > MaxValue)
                value = MaxValue;

            this.value = value;
        }
    }
}

Would you write tests for this class and if so how would you write them?

I'm thinking especially about the constructors. Like, would you have one test that created a a Number using the default constructor and checking that the value was 0, minvalue was 0 and maxvalue was 100? Or would that be over specification? Or is it really not, since others could depend on that the default values did not change by accident? Would you write a test for each constructor, or just of the default one since you know it chains through all the others.

+2  A: 

I have completely switched from the classic approach for TDD to the more modern and logical BDD (Behavior Driven Design). In the case of your Number class, I would write the following BDD Specifications (Note that the syntax below is done with SubSpec, which relies on xUnit.NET):

public void Parameterless_constructor_initializes_all_defaults_properly()
{
    // State
    Number number = null;

    // Context
    "Given a null context".Context(() => {});

    // Concern
    "when creating a new Number with no parameters".Do(() => { number = new Number(); });

    // Observations
    "the Value property should contain the default value 0".Assert(() => Assert.Equal(0, number.value));
    "the MinValue property should be 0".Assert(() => Assert.Equal(0, number.MinValue));
    "the MaxValue property should be 100".Assert(() => Assert.Equal(100, number.MaxValue));
}

public void Single_parameter_constructor_initializes_all_defaults_and_initial_value_properly()
{
    // State
    Number number = null;

    // Context
    "Given a null context".Context(() => {});

    // Concern
    "when creating a new Number with the initial value".Do(() => { number = new Number(10); });

    // Observations
    "the Value property should contain the value 10".Assert(() => Assert.Equal(10, number.value));
    "the MinValue property should be 0".Assert(() => Assert.Equal(0, number.MinValue));
    "the MaxValue property should be 100".Assert(() => Assert.Equal(100, number.MaxValue));
}

public void Full_constructor_initializes_all_values_properly()
{
    // State
    Number number = null;

    // Context
    "Given a null context".Context(() => {});

    // Concern
    "when creating a new Number with the initial, min, and max values".Do(() => { number = new Number(10, 1, 50); });

    // Observations
    "the Value property should contain the value 10".Assert(() => Assert.Equal(10, number.value));
    "the MinValue property should be 1".Assert(() => Assert.Equal(1, number.MinValue));
    "the MaxValue property should be 50".Assert(() => Assert.Equal(50, number.MaxValue));
}

In addition, I noticed that you also have a possible exceptional scenario for your full constructor, when the min value is greater than the max value. You would also want to verify proper behavior in this exceptional case:

public void Full_constructor_throws_proper_exception_when_minvalue_greater_than_maxvalue()
{
    // State
    Number number = null;
    Exception expectedEx = null;

    // Context
    "Given a null context".Context(() => {});

    // Concern
    "when creating a new Number with inverted min and max values".Do(
        () => 
        { 
            try { number = new Number(10, 50, 1); }
            catch (Exception ex) { expectedEx = ex }
        }
     );

    // Observations
    "an exception should be thrown".Assert(() => Assert.NotNull(expectedEx));
    "the exception should be an ArgumentException".Assert(() => Assert.IsType<ArgumentException>(expectedEx));
}

The above specifications should give you 100% test coverage. They also produce a very nice, human readable, logical report when executed with xunit.net and output the default report.

jrista
What's the null context for?
Svish
Well, there really isn't any specific context. The behavior executes properly in any context, it doesn't require anything special. If you relied on some specific context...for example, you required one or more dependencies to be created and injected, then you would prepare them as part of context. You could rewrite the tests to remove the null context part alltogether, and simply construct the Number class in the context, and eliminate the .Do() part. You could end up with: "Given a new number initialized with no parameters, the Value property should contain the default value 0."
jrista
I threw in the null context to demonstrate the full intent of BDD and how to use SubSpec. ;-) Sorry if it caused confusion.
jrista
A: 

Use nunit.

Create a test that creates an object for each constructor. Use the Assert.AreEqual to make sure all the objects are equal (You should override Equals for classes like this). To be extra sure, negative assert the Assert.AreSame assertion.

Then test each property for correct value.

If your class was more complicated and you wanted to be even more careful, you could then set all the values to unique random numbers and assert that the properties are initialized correctly from a random dataset.

Bob
+1  A: 

I guess you have several constructors for a reason - try to test scenario and not that the class was initialized according to some rule.
For example if you use the default constructor to create a class for on the fly calculation test that and not the fact that the default constructor has some value set.

My point is that you should not have overloads you do not use (unless you're developing API) so why not test the use case instead of the constructor.

Dror Helper
A: 

I would write a unit test for each constructor, checking that the minimum and maximum values are set properly. I would do this to make sure that if I change the code of one of the constructors later, my tests tell me what changed where.
I would also extract the default min and max into a constant, probably, so that the test would look like Assert.AreEqual(DefaultMinimum, myNumber.MinValue).
I would have a test checking that an invalid min/max throws an exception.
And I would rename that class "BoundedNumber" or something along these lines :)

Mathias
A: 

OK, I'm really answering the question right below the code listing (rather than the one in the headline)...

I think the main value of this class is in its (pun not intended) Value property. Therefore, it is that property rather than the constructors that should be the focus of unit tests.

If you write unit tests after you've written the three constructors and make those tests too restrictive (over-specification), then you risk ending up with a suite of brittle, hard-to-maintain tests.

azheglov