views:

364

answers:

5

Hello,

I have a quick little application and wanted to give a try to develop using TDD. I've never used TDD and actually didn't even know what it was until I found ASP.NET-MVC. (My first MVC app had unit tests, but they were brittle, way coupled, took too much up keep, and were abandoned -- I've come to learn unit tests != TDD).

Background on app:

I have a text dump of a purchase order read in as a string. I need to parse the text and return new purchase order number, new line item number, old purchase order number, old purchase order line number. Pretty simple.

Right now I'm only working on new purchase order details (number/line) and have a model like this:

public class PurchaseOrder
{
    public string NewNumber {get; private set;}
    public string NewLine {get; private set;}

    new public PurchaseOrder(string purchaseOrderText)
    {
        NewNumber = GetNewNumber(purchaseOrderText);
        NewLine = GetNewLine(purchaseOrderText);
    }

    // ... definition of GetNewNumber / GetNewLine ...
    //  both return null if they can't parse the text
}

Now I want to add a method "IsValid" that should only be true if "NewNumber" and "NewLine" are both non-null. So I want to test it like:

public void Purchase_Order_Is_Valid_When_New_Purchase_Order_Number_And_Line_Number_Are_Not_Null()
{
    PurchaseOrder order = new PurchaseOrder()
    {
        NewNumber = "123456",
        NewLine = "001"
    };

    Assert.IsTrue(order.IsValid);
}

This is easy enough, but it seems like a bad compromise to allow public setters and a parameterless constructor. So the alternative is to feed in a 'purchaseOrderText' value in the constructor, but then I'm testing the code for 'GetNewNumber' and 'GetNewLine' as well.

I'm kind of stumped on how to write this as a testable class while trying to keep it locked up in terms of what makes sense for the model. This seems like it would be a common problem, so I'm thinking I'm just missing an obvious concept.

+2  A: 

Instead of making the setters public, make them internal and then make your test assembly InternalsVisibleTo in your main project. That way, your tests can see your internal members, but no-one else can.

In you main project, put something like this;

[assembly: InternalsVisibleTo( "UnitTests" )]

Where UnitTests is the name of your test assembly.

Rob Prouse
+6  A: 

One solution is to not have the constructor do the work:

public class PurchaseOrder
{
    public PurchaseOrder(string newNumber, string newLine)
    {
        NewNumber = newNumber;
        NewLine = newLine;
    }
    // ...
}

Then testing is easy and isolated - you're not testing GetNewNumber and GetNewLine at the same time.

To help using PurchaseOrder you can create a factory method that puts it together:

public static PurchaseOrder CreatePurchaseOrder(string purchaseOrderText)
{
   return new PurchaseOrder(
     GetNewNumber(purchaseOrderText),
     GetNewLine(purchaseOrderText));
}
orip
This also is a better use of the single responsibility principle -- the conversion from text to number and line item is essentially an IO function, not something that is inherent to a purchase order. If the format of your text dump changes, your purchase order shouldn't need to.
JacobM
And I'll add that this is exactly the kind of design realization that TDD is great at revealing -- if you can't make a reasonable test of something, it's probably doing too much.
JacobM
Thanks, for answer and comments. I agree with both and must admit I am kind of giddy to see TDD do this (works as advertised). This program is real simple, could be done just as 1 procedural function in main, but I figured you have to learn by doing instead of just reading about it.
eyston
+1  A: 

Create a private accessor for your class in the test project and then use the accessor to set the properties for your test. In VS you can create private accessors by right-clicking in the class, choosing Create Private Accessor, and selecting the test project. In your test you then use it like so:

public void NameOfTest()
{
    PurchaseOrder_Accessor order = new PurchaseOrder_Accessor();
    order.NewNumber = "123456";
    order.NewLine = "001";
    Assert.IsTrue(order.IsValid);
}

If you have a default constructor you can do:

public void NameOfTest()
{
    PurchaseOrder order = new PurchaseOrder()
    PurchaseOrder_Accessor accessor =
                new PurchaseOrder_Accessor( new PrivateObject(order) );
    accessor.NewNumber = "123456";
    accessor.NewLine = "001";
    Assert.IsTrue(order.IsValid);
}
tvanfosson
Wow, I didn't even know about Accessors. Found MSDN article on them, and its on my todo list :) Thanks!
eyston
A: 

I would probably create a new constructor public PurchaseOrder(string newNumber, string newLine). IMO, you're likely to need it anyway.

trendl
A: 

a little offtopic :)

Using TDD, it`s about writing your unit-tests first, then just write enough code to pass the test and after that do refactoring. By writing your tests first you should exactly know what and maybe how to program to make your code testable!

just my 2 cents...

cordellcp3
I am attempting this. My next test was the IsValid one, which is an easy test to write. After I wrote the test it is obvious that the code as I had it could not support the test, so I either needed to refactor, or rewrite the test (but I didn't know which). Thanks!
eyston