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.