Sunday, July 03, 2005

Focus on readability and avoid being SetUp

The SetUp method is used to initialize a test prior to running it. However, there are disadvantages to using SetUp that should be noted. Christian Taubman pointed out a few of the disadvantages in a previous entry. The focus of his entry is replacing SetUp with private methods. While I agree with this, the focus of my entry is more about readability.

Assume you are working with a class:

public class Reservation
{
public Reservation(DateTime checkInDate, int nights, int guests)
{
this.checkInDate = checkInDate;
this.nights = nights;
this.guests = guests;
}

private DateTime checkInDate;
private int nights;
private int guests;
public event ErrorEventHandler ReservationBeforeToday;
public event ErrorEventHandler InvalidNights;
public event ErrorEventHandler InvalidGuests;
public delegate void ErrorEventHandler();

public void Validate()
{
if (checkInDate<DateTime.Today) ReservationBeforeToday();
if (nights<1) InvalidNights();
if (guests<1) InvalidGuests();
}
}

There should be tests for the logic in Validate. The test could look like this:

public class ReservationTests
{
private static bool eventRaised;
private static Reservation reservation;

[SetUp]
public void SetUp()
{
eventRaised = false;
reservation = new Reservation(DateTime.AddDays(-2),0,0);
}

[Test]
public void InvalidReservationRaisesEvent()
{
reservation.ReservationBeforeToday+=delegate { eventRaised = true; };
reservation.Validate();
}

[Test]
public void InvalidNightsRaisesEvent()
{
reservation.InvalidNights+=delegate { eventRaised = true; };
reservation.Validate();
}

[Test]
public void InvalidGuestsRaisesEvent()
{
reservation.InvalidGuests+=delegate { eventRaised = true; };
reservation.Validate();
}

[TearDown]
public void TearDown()
{
Assert.IsTrue(eventRaised);
}
}

While this code does accomplish what I was looking for I don't consider it to be very readable. An alternative which is much more readable could look like:

public class ReservationTests
{
private static bool eventRaised;
private static Reservation reservation;

public void Init()
{
eventRaised = false;
reservation = new Reservation(DateTime.AddDays(-2),0,0);
}

[Test]
public void InvalidReservationRaisesEvent()
{
Init();
reservation.ReservationBeforeToday+=delegate { eventRaised = true; };
reservation.Validate();
Assert.IsTrue(eventRaised);
}

[Test]
public void InvalidNightsRaisesEvent()
{
Init();
reservation.InvalidNights+=delegate { eventRaised = true; };
reservation.Validate();
Assert.IsTrue(eventRaised);
}

[Test]
public void InvalidGuestsRaisesEvent()
{
Init();
reservation.InvalidGuests+=delegate { eventRaised = true; };
reservation.Validate();
Assert.IsTrue(eventRaised);
}
}

However, the most readable version is my favorite:

public class ReservationTests
{
[Test]
public void InvalidReservationRaisesEvent()
{
bool eventRaised = false;
Reservation reservation = createReservation();
reservation.ReservationBeforeToday+=delegate { eventRaised = true; };
reservation.Validate();
Assert.IsTrue(eventRaised);
}

[Test]
public void InvalidNightsRaisesEvent()
{
bool eventRaised = false;
Reservation reservation = createReservation();
reservation.InvalidNights+=delegate { eventRaised = true; };
reservation.Validate();
Assert.IsTrue(eventRaised);
}

[Test]
public void InvalidGuestsRaisesEvent()
{
bool eventRaised = false;
Reservation reservation = createReservation();
reservation.InvalidGuests+=delegate { eventRaised = true; };
reservation.Validate();
Assert.IsTrue(eventRaised);
}

private Reservation createReservation()
{
return new Reservation(DateTime.AddDays(-2),0,0);
}
}

The result is a test that can be read almost entirely in isolation and understood easily.

1 comment:

  1. I'd rather have one method called something like InvalidStateRaisesEvent(DelegateInvoker invoker) where DelegateInvoker is an interface and then have each test call this method with an anonymous class implementation. I'm sure there's a better way to do this in .NET.

    In any case, that creates one example of the common procedure and allows me to easily check whether I have all the data combinations covered.

    Takes care of duplication and I would say even more readable.

    ReplyDelete

Note: Only a member of this blog may post a comment.