Sunday, June 30, 2013

EF Code First: seeding with foreign keys

This is just a quick tip, more for future reference for myself than anything else.

When using Entity Framework Code First, one of the things you might want to do while developing the application is to seed your database with some test data. This can be something as easy as:

    protected override void Seed(InventoryDB context)
    {
      context.Products.AddOrUpdate(
        p => p.Name,
        new Product { Name = "Hammer", SalePrice = 11.99m, },
        new Product { Name = "Nail Pack x100", SalePrice = 0.05m, },
        new Product { Name = "Saw", SalePrice = 19.99m, },
        new Product { Name = "Toolkit", SalePrice = 39.99m, }
        );
      //...
    }

(I am using the AddOrUpdate method so the code detects whether those products already exist and doesn't add them again if they do.)

The Product class is quite simple and – most importantly – doesn't depend on any other:

public class Product
  {
    public int Id { get; set; }

    [StringLength(256)]
    public string Name { get; set; }

    public decimal? SalePrice { get; set; }
  }

However, with a more complex relationship like this one things get complicated:

  public class Acquisition
  {
    public int Id { get; set; }
    public int CompanyId { get; set; }

    public virtual ICollection<AcquisitionItem> Items { get; set; }
    public virtual Company Company { get; set; }
  }

  public class AcquisitionItem
  {
    public int Id { get; set; }
    public int AcquisitionId { get; set; }
    public int ProductId { get; set; }
    public decimal Quantity { get; set; }
    public decimal Price { get; set; }

    public virtual Acquisition Acquisition { get; set; }
    public virtual Product Product { get; set; }
  }

The problem occurs when I try to set the CompanyId and ProductId properties:

      context.Acquisitions.AddOrUpdate(
        new Acquisition
        {
          CompanyId = ?
          Items = new[]
          {
            new AcquisitionItem { ProductId = ?, Quantity = 20, Price = 5.99m },
            new AcquisitionItem { ProductId = ?, Quantity = 2000, Price = 0.01m },
          }
        }
      );

(The EF library is "smart" enough to automatically associate the AcquisitionItems with their parent Acquisition entity, which is great – otherwise this code would have been a lot more complicated.)

The solution is to add the products (and companies) first, call the SaveChanges method and then get the Ids of the entities I'm going to use; this is the complete Seed method:

    protected override void Seed(InventoryDB context)
    {
      //  This method will be called after migrating to the latest version.

      context.Products.AddOrUpdate(
        p => p.Name,
        new Product { Name = "Hammer", SalePrice = 11.99m, },
        new Product { Name = "Nail Pack x100", SalePrice = 0.05m, },
        new Product { Name = "Saw", SalePrice = 19.99m, },
        new Product { Name = "Toolkit", SalePrice = 39.99m, }
        );
      context.Companies.AddOrUpdate(
        c => c.Name,
        new Company { Name = "Acme" },
        new Company { Name = "Hotpoint" }
        );

      context.SaveChanges();

      // get the name -> id mappings
      var products = context
        .Products
        .ToDictionary(p => p.Name, p => p.Id);
      var companies = context
        .Companies
        .ToDictionary(c => c.Name, c => c.Id);

      context.Acquisitions.AddOrUpdate(
        new Acquisition
        {
          CompanyId = companies["Acme"],
          Items = new[]
          {
            new AcquisitionItem { ProductId = products["Hammer"], Quantity = 20, Price = 5.99m },
            new AcquisitionItem { ProductId = products["Nail Pack x100"], Quantity = 2000, Price = 0.01m },
          }
        },
        new Acquisition
        {
          CompanyId = companies["Hotpoint"],
          Items = new[]
          {
            new AcquisitionItem { ProductId = products["Saw"], Quantity = 10, Price = 12.99m },
            new AcquisitionItem { ProductId = products["Toolkit"], Quantity = 10, Price = 29.99m },
          }
        }
        );

      context.SaveChanges();
    }

I used the ToDictionary extension method to create name-to-Id mappings for products and companies, which allowed me to avoid using a construct like:

            new AcquisitionItem { ProductId = Products.FirstOrDefault(p => p.Name == "Saw").Id, Quantity = 10, Price = 12.99m },

which would have worked but would have been much uglier.

(This was taken from my Inventory project up on GitHub.)

Wednesday, June 19, 2013

Comparing Excel files, take two

I already wrote this project once, on GitHub, but I can't say that I like the result. I wrote that code without TDD, mostly to see if I can still work in the "normal" fashion. I can, but I had a bug that took me a while to even discover and then was difficult to write tests for. I also hate the temporal dependency in that design (if you don't call the SortBy method first, the ExcludeRecords method will return incorrect results).

Anyway; here's attempt number two, writing that code using TDD.

Structure

I start by creating my usual structure for projects of this type - a library for the logic, a console application for the main program and a test project for tests. I also use the Enable NuGet Package Restore option (right-click on the solution node) to add the three NuGet files that will allow someone to automatically download all the packages I'm referencing without wasting space in the source code repository. (Right now I've only added the Moq package to the test project - I'm pretty much guaranteed to use that one.)

Here's what the solution tree looks like:

Solution Tree

Acceptance tests

I am fine with how the previous program worked, from the point of view of the end-user; what I dislike is the internal structure. That means that the acceptance test will look the same: launch the program with the correct arguments, capture its output, verify that the output matches the desired result:

  [TestClass]
  public class AcceptanceTests
  {
    [TestMethod]
    public void EndToEnd()
    {
      const string PATH = @"..\..\..\CompareExcelFiles\bin\Debug\CompareExcelFiles.exe";
      const string ARGS = @"..\..\..\file1.xlsx ..\..\..\file2.xlsx C A";
      const string EXPECTED_OUTPUT = @"** 1: ..\..\..\file1.xlsx ** --- 3 distinct rows out of 9
      C A
00001 2 1
00002 2 2
00003 2 3

** 2: ..\..\..\file2.xlsx ** --- 2 distinct rows out of 8
      C A
00001 1 4
00002 3 4

";

      var startInfo = new ProcessStartInfo
      {
        CreateNoWindow = true,
        RedirectStandardInput = true,
        RedirectStandardOutput = true,
        UseShellExecute = false,
        Arguments = ARGS,
        FileName = PATH,
      };
      var process = new Process { StartInfo = startInfo };
      process.Start();
      process.WaitForExit();

      var result = process.StandardOutput.ReadToEnd();

      Assert.AreEqual(EXPECTED_OUTPUT, result);
    }
  }

This fails, as expected. Time to think about the design of the program. What do I want the main program to do?

Well, it needs to read the name of two Excel files from the program arguments and then at least one column name. It needs to load the two files and try to match them by comparing the given column(s). Finally, it needs to display the non-matching rows for each files (the rows that exist in one file but have no correspondent in the other), again based only on the values in the given column(s).

One thing that comes to mind is: what happens if the program receives too few arguments? I've decided that, instead of asking for them, the program will just display a help message describing its purpose and syntax. In fact, this suggests a second acceptance test:

    [TestMethod]
    public void ReturnsHelp()
    {
      const string PATH = @"..\..\..\CompareExcelFiles\bin\Debug\CompareExcelFiles.exe";
      const string EXPECTED_OUTPUT = @"Syntax: CompareExcelFiles file1 file2 column [column...]
        file1     first file to compare
        file2     second file to compare
        column    name of column(s) to sort / compare by
";

      var startInfo = new ProcessStartInfo
      {
        CreateNoWindow = true,
        RedirectStandardInput = true,
        RedirectStandardOutput = true,
        UseShellExecute = false,
        Arguments = "",
        FileName = PATH,
      };
      var process = new Process { StartInfo = startInfo };
      process.Start();
      process.WaitForExit();

      var result = process.StandardOutput.ReadToEnd();

      Assert.AreEqual(EXPECTED_OUTPUT, result);
    }

This fails. I know it cries for a refactoring (it's almost entirely a copy of the first acceptance test) but I must resist. Don't refactor a failing test. First, make it pass, which is rather easy in this case:

  internal class Program
  {
    private static void Main(string[] args)
    {
      Console.WriteLine("Syntax: CompareExcelFiles file1 file2 column [column...]");
      Console.WriteLine("        file1     first file to compare");
      Console.WriteLine("        file2     second file to compare");
      Console.WriteLine("        column    name of column(s) to sort / compare by");
    }
  }

That acceptance test is green. Now I can refactor the tests:

  [TestClass]
  public class AcceptanceTests
  {
    [TestMethod]
    public void EndToEnd()
    {
      const string ARGS = @"..\..\..\file1.xlsx ..\..\..\file2.xlsx C A";
      const string EXPECTED_OUTPUT = @"** 1: ..\..\..\file1.xlsx ** --- 3 distinct rows out of 9
      C A
00001 2 1
00002 2 2
00003 2 3

** 2: ..\..\..\file2.xlsx ** --- 2 distinct rows out of 8
      C A
00001 1 4
00002 3 4

";

      var result = RunAndCaptureOutput(ARGS);

      Assert.AreEqual(EXPECTED_OUTPUT, result);
    }

    [TestMethod]
    public void ReturnsHelp()
    {
      const string EXPECTED_OUTPUT = @"Syntax: CompareExcelFiles file1 file2 column [column...]
        file1     first file to compare
        file2     second file to compare
        column    name of column(s) to sort / compare by
";
      var result = RunAndCaptureOutput("");

      Assert.AreEqual(EXPECTED_OUTPUT, result);
    }

    //

    private static string RunAndCaptureOutput(string args)
    {
      const string PATH = @"..\..\..\CompareExcelFiles\bin\Debug\CompareExcelFiles.exe";

      var startInfo = new ProcessStartInfo
      {
        CreateNoWindow = true,
        RedirectStandardInput = true,
        RedirectStandardOutput = true,
        UseShellExecute = false,
        Arguments = args,
        FileName = PATH,
      };
      var process = new Process { StartInfo = startInfo };
      process.Start();
      process.WaitForExit();

      return process.StandardOutput.ReadToEnd();
    }
  }

The first acceptance test fails, the second passes. Good; time for the unit tests.

Unit tests

As I said, I want to load the two Excel files whose name I got as arguments. What is the result of this action? Because I know that I intend to compare the contents of these files, they will have to be basically tables; because I'm passing column names, the first row will be the header row. That means a bi-dimensional structure, with the column names as a separate property for easy reference. Oh, another thing: I have no need to modify the values so the property exposing them should be read-only.

I'll start by creating the interface expressing these concepts (of course, I can always change it later if I discover something I haven't planned for); add this to the library project:

  public interface Table
  {
    int RowCount { get; }
    int ColCount { get; }

    string[] Columns { get; }
    string[][] Data { get; }
  }

So far so good. Now I need a class that can load an Excel file (actually, a sheet from an Excel file) into such a structure. Unfortunately, testing such a class involves actually opening an Excel file, which means it doesn't qualify as a unit test (see this set of rules). It's ok, I'll add this to the AcceptanceTests class:

    [TestMethod]
    public void LoadsExcelFile()
    {
      const string FILE_NAME = @"..\..\..\file1.xlsx";

      var sut = new ExcelLoader();

      var result = sut.Load(FILE_NAME);

      Assert.AreEqual(9, result.RowCount); // number of *data* rows
      Assert.AreEqual(4, result.ColCount);
      CollectionAssert.AreEqual(new[] { "A", "B", "C", "D" }, result.Columns);
      CollectionAssert.AreEqual(new[] { "1", "10", "1", "100" }, result.Data[0]);
      CollectionAssert.AreEqual(new[] { "2", "20", "1", "200" }, result.Data[1]);
      CollectionAssert.AreEqual(new[] { "3", "30", "1", "300" }, result.Data[2]);
      CollectionAssert.AreEqual(new[] { "1", "40", "2", "400" }, result.Data[3]);
      CollectionAssert.AreEqual(new[] { "2", "50", "2", "500" }, result.Data[4]);
      CollectionAssert.AreEqual(new[] { "3", "60", "2", "600" }, result.Data[5]);
      CollectionAssert.AreEqual(new[] { "1", "70", "3", "700" }, result.Data[6]);
      CollectionAssert.AreEqual(new[] { "2", "80", "3", "800" }, result.Data[7]);
      CollectionAssert.AreEqual(new[] { "3", "90", "3", "900" }, result.Data[8]);
    }

Of course, this doesn't even compile; I need to add the ExcelLoader class to the library project:

  public class ExcelLoader
  {
    public Table Load(string fileName)
    {
      return null;
    }
  }

Now the code compiles and the test fails.

MemoryTable

Unfortunately, the test fails for the wrong reason: I'm returning null. I need to return an implementation of the Table interface but with the wrong data; I'll call it MemoryTable. Of course, I need a test first; I've decided that the MemoryTable class will be initialized with a list of string arrays, so that's what I'll use in my test:

  [TestClass]
  public class MemoryTableTests
  {
    [TestClass]
    public class RowCount : MemoryTableTests
    {
      [TestMethod]
      public void SingleRow()
      {
        var sut = new MemoryTable(new[]
        {
          new[] { "A", "B" },
          new[] { "1", "2" },
        });

        Assert.AreEqual(1, sut.RowCount);
      }
    }
  }

Making it compile is easy

  public class MemoryTable : Table
  {
    public int RowCount { get; private set; }
    public int ColCount { get; private set; }
    public string[] Columns { get; private set; }
    public string[][] Data { get; private set; }

    public MemoryTable(IEnumerable<string[]> cells)
    {
      //
    }
  }

Making it pass is not that complicated either (yes, I know it looks ridiculous; doing it this way forces me to really test the class):

    public MemoryTable(IEnumerable<string[]> cells)
    {
      RowCount = 1;
    }

Ok, the second test needs to force me to change that code:

      [TestMethod]
      public void MultipleRows()
      {
        var sut = new MemoryTable(new[]
        {
          new[] { "A", "B" },
          new[] { "1", "2" },
          new[] { "3", "4" },
          new[] { "5", "6" },
        });

        Assert.AreEqual(3, sut.RowCount);
      }

The fix:

    public MemoryTable(IEnumerable<string[]> cells)
    {
      RowCount = cells.Count() - 1;
    }

That code raises two questions. First, what happens if the cells enumeration is empty? I should throw an exception in that case, since I need at least one row (the header):

      [TestMethod]
      [ExpectedException(typeof (Exception))]
      public void ThrowsWhenNoRows()
      {
        var sut = new MemoryTable(new List<string[]>());
      }
%

The test fails because I don't throw an exception; easy to fix:

[%
    public MemoryTable(IEnumerable<string[]> cells)
    {
      cells = cells.ToList();
      if (!cells.Any())
        throw new Exception("At least one row is required.");

      RowCount = cells.Count() - 1;
    }

(I call the ToList method because I don't want to enumerate cells twice - once for Any and once for Count.)

The second question is: what happens if I send a null argument? I say that it should be treated the same as an empty sequence:

      [TestMethod]
      [ExpectedException(typeof(Exception))]
      public void ThrowsWhenNull()
      {
        var sut = new MemoryTable(null);
      }

The method fails because the exception being thrown is an ArgumentNullException instead of Exception. I think a regular Exception (in fact, the same one as for an non-null but empty sequence) is still the correct thing to do:

    public MemoryTable(IEnumerable<string[]> cells)
    {
      cells = (cells ?? new List<string[]>()).ToList();
      if (!cells.Any())
        throw new Exception("At least one row is required.");

      RowCount = cells.Count() - 1;
    }

Ok, that's it for the RowCount property. Here are the similar tests for ColCount:

    [TestClass]
    public class ColCount : MemoryTableTests
    {
      [TestMethod]
      public void SingleColumn()
      {
        var sut = new MemoryTable(new[]
        {
          new[] { "A" },
          new[] { "1" },
          new[] { "2" },
        });

        Assert.AreEqual(1, sut.ColCount);
      }

      [TestMethod]
      public void MultipleColumns()
      {
        var sut = new MemoryTable(new[]
        {
          new[] { "A", "B", "C" },
          new[] { "1", "2", "3" },
          new[] { "4", "5", "6" },
        });

        Assert.AreEqual(3, sut.ColCount);
      }

      [TestMethod]
      [ExpectedException(typeof (Exception))]
      public void ThrowsWhenNoColumns()
      {
        var sut = new MemoryTable(new[]
        {
          new string[0],
          new string[0],
        });
      }
    }

(Note that this is an inner class for MemoryTableTests.)

The code for the MemoryTable constructor is now

    public MemoryTable(IEnumerable<string[]> cells)
    {
      cells = (cells ?? new List<string[]>()).ToList();

      RowCount = cells.Count() - 1;
      if (RowCount < 0)
        throw new Exception("At least one row is required.");

      ColCount = cells.First().Length;
      if (ColCount < 1)
        throw new Exception("At least one column is required.");
    }

Finally, these are the tests for the Columns and Data properties:

    [TestClass]
    public class Columns : MemoryTableTests
    {
      [TestMethod]
      public void SingleColumn()
      {
        var sut = new MemoryTable(new[]
        {
          new[] { "A" },
          new[] { "1" },
          new[] { "2" },
        });

        CollectionAssert.AreEqual(new[] { "A" }, sut.Columns);
      }

      [TestMethod]
      public void MultipleColumns()
      {
        var sut = new MemoryTable(new[]
        {
          new[] { "A", "B", "C" },
          new[] { "1", "2", "3" },
          new[] { "4", "5", "6" },
        });

        CollectionAssert.AreEqual(new[] { "A", "B", "C" }, sut.Columns);
      }
    }

    [TestClass]
    public class Data : MemoryTableTests
    {
      [TestMethod]
      public void MultipleRowsAndColumns()
      {
        var sut = new MemoryTable(new[]
        {
          new[] { "A", "B", "C" },
          new[] { "1", "2", "3" },
          new[] { "4", "5", "6" },
        });

        Assert.AreEqual("1", sut.Data[0][0]);
        Assert.AreEqual("2", sut.Data[0][1]);
        Assert.AreEqual("3", sut.Data[0][2]);
        Assert.AreEqual("4", sut.Data[1][0]);
        Assert.AreEqual("5", sut.Data[1][1]);
        Assert.AreEqual("6", sut.Data[1][2]);
      }
    }

and the MemoryTable constructor is now

  public class MemoryTable : Table
  {
    public int RowCount { get; private set; }
    public int ColCount { get; private set; }
    public string[] Columns { get; private set; }
    public string[][] Data { get; private set; }

    public MemoryTable(IEnumerable<string[]> cells)
    {
      cells = (cells ?? new List<string[]>()).ToList();

      RowCount = cells.Count() - 1;
      if (RowCount < 0)
        throw new Exception("At least one row is required.");

      Columns = cells.First();
      ColCount = Columns.Length;
      if (ColCount < 1)
        throw new Exception("At least one column is required.");

      Data = cells.Skip(1).ToArray();
    }
  }

Whew.

ExcelLoader

Ok, back to the ExcelLoader class. Let's see if I can make the test fail correctly now:

  public class ExcelLoader
  {
    public Table Load(string fileName)
    {
      return new MemoryTable(new[]
      {
        new[] { "A" },
      });
    }
  }

I run the tests and, indeed, the acceptance test is now failing with the message "Assert.AreEqual failed. Expected:<9>. Actual:<0>.". It's better when a test fails because of an assertion than because of some unexpected error.

To make the test pass, I first need to add the EPPlus package to the library project. At the time when I write this, the last version of that package is 3.1.3.3 so be careful if you're using another one (especially if the major version is different).

After that, change the code of the ExcelLoader class to

  public class ExcelLoader
  {
    public Table Load(string fileName)
    {
      return new MemoryTable(ReadExcel(fileName));
    }

    //

    private static IEnumerable<string[]> ReadExcel(string fileName)
    {
      using (var file = new FileStream(fileName, FileMode.Open, FileAccess.Read, FileShare.Read))
      using (var excel = new ExcelPackage(file))
      {
        var sheet = excel.Workbook.Worksheets[1];
        var lastRow = sheet.Dimension.End.Row;
        var lastCol = sheet.Dimension.End.Column;

        for (var row = 1; row <= lastRow; row++)
        {
          var record = new List<string>();

          for (var col = 1; col <= lastCol; col++)
            record.Add(sheet.Cells[row, col].GetValue<string>() ?? "");

          yield return record.ToArray();
        }
      }
    }
  }

The second acceptance test is now passing; I can read Excel files.

Comparing tables

I have now reached the "compare the two tables" part of the exercise. I'll create a new TableComparer class, starting with a test:

  [TestClass]
  public class TableComparerTests
  {
    [TestClass]
    public class Compare : TableComparerTests
    {
      [TestMethod]
      public void ReturnsAllRowsWhenTheOtherTableIsEmpty()
      {
        var table1 = new MemoryTable(new[] { new[] { "A", "B" }, new[] { "1", "2" }, new[] { "3", "4" } });
        var table2 = new MemoryTable(new[] { new[] { "A", "B" } });
        var sut = new TableComparer(new[] { "A" });

        var result = sut.Compare(table1, table2);

        Assert.AreEqual(2, result.RowCount);
      }
    }
  }

To make it compile, add a new class to the library project:

  public class TableComparer
  {
    public TableComparer(string[] columns)
    {
      //
    }

    public Table Compare(Table table1, Table table2)
    {
      return null;
    }
  }

This fails; making it pass is easy - just return the first table:

    public Table Compare(Table table1, Table table2)
    {
      return table1;
    }

That means I need a new test:

      [TestMethod]
      public void ReturnsRowsThatDifferInOneColumn()
      {
        var table1 = new MemoryTable(new[]
        {
          new[] { "A", "B" },
          new[] { "1", "2" },
          new[] { "3", "4" },
          new[] { "5", "6" },
          new[] { "7", "8" },
        });
        var table2 = new MemoryTable(new[]
        {
          new[] { "A", "B" },
          new[] { "1", "2" },
          new[] { "5", "6" },
        });
        var sut = new TableComparer(new[] { "A" });

        var result = sut.Compare(table1, table2);

        Assert.AreEqual(2, result.RowCount);
        CollectionAssert.AreEqual(new[] { "3", "4" }, result.Data[0]);
        CollectionAssert.AreEqual(new[] { "7", "8" }, result.Data[1]);
      }

Making this pass takes a bit more work than I usually need on a test:

  public class TableComparer
  {
    public TableComparer(string[] columns)
    {
      this.columns = columns;
    }

    public Table Compare(Table table1, Table table2)
    {
      var indices = GetIndices(table1.Columns).ToList();

      var rows = table1
        .Data
        .Where(row => !RowFoundIn(row, table2.Data, indices));

      return new MemoryTable(Enumerable.Repeat(table1.Columns, 1).Concat(rows));
    }

    //

    private readonly string[] columns;

    private IEnumerable<int> GetIndices(string[] tableColumns)
    {
      return columns.Select(col => Array.IndexOf(tableColumns, col));
    }

    private static bool RowFoundIn(IList<string> row, IEnumerable<string[]> data, IEnumerable<int> indices)
    {
      // can't use .FirstOrDefault() here, the default is 0
      var comparisons = data
        .Select(otherRow => CompareRows(row, otherRow, indices))
        .Where(comparison => comparison == 0)
        .Take(1)
        .ToList();

      return comparisons.Any();
    }

    private static int CompareRows(IList<string> row1, IList<string> row2, IEnumerable<int> indices)
    {
      return indices
        .Select(index => string.Compare(row1[index], row2[index], StringComparison.InvariantCultureIgnoreCase))
        .Where(comparison => comparison != 0)
        .FirstOrDefault();
    }
  }

This is quite inefficient – O(n2), where n is the total number of elements in a table, that is rows * columns – but I don't need to optimize until I discover that that's indeed a problem. I'm more interested in making it work first. (The Make it work, make it right, make it fast mantra comes to mind here.)

What else should I test? I believe I already made the algorithm work for multiple columns, but making sure can't hurt:

      [TestMethod]
      public void ReturnsRowsThatDifferInMultipleColumns()
      {
        var table1 = new MemoryTable(new[]
        {
          new[] { "A", "B" },
          new[] { "1", "0" },
          new[] { "1", "2" },
          new[] { "1", "4" },
          new[] { "1", "6" },
          new[] { "1", "8" },
        });
        var table2 = new MemoryTable(new[]
        {
          new[] { "A", "B" },
          new[] { "1", "2" },
          new[] { "2", "0" },
          new[] { "1", "0" },
        });
        var sut = new TableComparer(new[] { "A", "B" });

        var result = sut.Compare(table1, table2);

        Assert.AreEqual(3, result.RowCount);
        CollectionAssert.AreEqual(new[] { "1", "4" }, result.Data[0]);
        CollectionAssert.AreEqual(new[] { "1", "6" }, result.Data[1]);
        CollectionAssert.AreEqual(new[] { "1", "8" }, result.Data[2]);
      }

Yep; this one passes too.

Writing out the results

All that remains to be done is the part where I display the results of the comparisons. The main part of that will be done by the MemoryTable class, which means a new inner class for MemoryTableTests. I'll start by verifying the header:

    [TestClass]
    public class Dump
    {
      [TestMethod]
      public void PrintsOutTheHeader()
      {
        var sut = new MemoryTable(new[]
        {
          new[] { "A", "B", "C" },
          new[] { "1", "2", "3" },
          new[] { "4", "5", "6" },
        });
        var output = new List<string>();

        sut.Dump(new[] { "A", "C" }, output.Add);

        Assert.AreEqual("      A C", output[0]);
      }
    }

A new method has to be added to the MemoryTable class to make this compile:

    public void Dump(string[] columns, Action<string> writeLine)
    {
      //
    }

Making it pass is not difficult:

    public void Dump(string[] columns, Action<string> writeLine)
    {
      writeLine(string.Format("      {0}", string.Join(" ", columns)));
    }

Good. Now the data:

      [TestMethod]
      public void PrintsOutTheData()
      {
        var sut = new MemoryTable(new[]
        {
          new[] { "A", "B", "C" },
          new[] { "1", "2", "3" },
          new[] { "4", "5", "6" },
        });
        var output = new List<string>();

        sut.Dump(new[] { "A", "C" }, output.Add);

        Assert.AreEqual(3, output.Count);
        Assert.AreEqual("00001 1 3", output[1]);
        Assert.AreEqual("00002 4 6", output[2]);
      }

Making it pass:

    public void Dump(string[] columns, Action<string> writeLine)
    {
      writeLine(string.Format("      {0}", string.Join(" ", columns)));

      var indices = GetIndices(columns).ToList();
      var lineNo = 1;

      var lines = Data.Select(row => string.Join(" ", indices.Select(index => row[index])));
      foreach (var line in lines)
        writeLine(string.Format("{0:d5} {1}", lineNo++, line));
    }

    //

    private IEnumerable<int> GetIndices(IEnumerable<string> requiredColumns)
    {
      return requiredColumns.Select(col => Array.IndexOf(Columns, col));
    }

Removing duplication

I will introduce a new class in order to remove the duplication; add this to the library project:

  public static class Helper
  {
    public static IEnumerable<int> GetIndices(this string[] allColumns, IEnumerable<string> requiredColumns)
    {
      return requiredColumns.Select(col => Array.IndexOf(allColumns, col));
    }
  }

The MemoryTable.Dump method becomes

    public void Dump(string[] columns, Action<string> writeLine)
    {
      writeLine(string.Format("      {0}", string.Join(" ", columns)));

      var indices = Columns.GetIndices(columns).ToList();
      var lineNo = 1;

      var lines = Data.Select(row => string.Join(" ", indices.Select(index => row[index])));
      foreach (var line in lines)
        writeLine(string.Format("{0:d5} {1}", lineNo++, line));
    }

and the TableComparer.Compare method changes to

    public Table Compare(Table table1, Table table2)
    {
      var indices = table1.Columns.GetIndices(columns).ToList();

      var rows = table1
        .Data
        .Where(row => !RowFoundIn(row, table2.Data, indices));

      return new MemoryTable(Enumerable.Repeat(table1.Columns, 1).Concat(rows));
    }

The two private GetIndices methods disappear and the unit tests are still green.

Main program

I've finally come to the changes I need to make to the main program to make the last failing test pass:

  internal class Program
  {
    private static void Main(string[] args)
    {
      if (args.Length < 3)
      {
        Console.WriteLine("Syntax: CompareExcelFiles file1 file2 column [column...]");
        Console.WriteLine("        file1     first file to compare");
        Console.WriteLine("        file2     second file to compare");
        Console.WriteLine("        column    name of column(s) to sort / compare by");

        return;
      }

      var excel = new ExcelLoader();
      var table1 = excel.Load(args[0]);
      var table2 = excel.Load(args[1]);

      var columns = args.Skip(2).ToArray();
      var comparer = new TableComparer(columns);

      var diff1 = comparer.Compare(table1, table2);
      var diff2 = comparer.Compare(table2, table1);

      DumpResult(1, args[0], diff1, table1.RowCount, columns);
      DumpResult(2, args[1], diff2, table2.RowCount, columns);
    }

    //

    private static void DumpResult(int fileIndex, string fileName, Table diff, int totalRows, string[] columns)
    {
      Console.WriteLine("** {0}: {1} ** --- {2} distinct rows out of {3}", fileIndex, fileName, diff.RowCount, totalRows);
      diff.Dump(columns, Console.WriteLine);
      Console.WriteLine();
    }
  }

I had to add the Dump method the Table interface to make this work:

  public interface Table
  {
    int RowCount { get; }
    int ColCount { get; }

    string[] Columns { get; }
    string[][] Data { get; }

    void Dump(string[] columns, Action<string> writeLine);
  }

The test… fails? Ah… when I wrote the acceptance test I had expected to sort the tables, which I never got to do. This time the test is wrong so I change it:

    [TestMethod]
    public void EndToEnd()
    {
      const string ARGS = @"..\..\..\file1.xlsx ..\..\..\file2.xlsx C A";
      const string EXPECTED_OUTPUT = @"** 1: ..\..\..\file1.xlsx ** --- 3 distinct rows out of 9
      C A
00001 2 1
00002 2 2
00003 2 3

** 2: ..\..\..\file2.xlsx ** --- 2 distinct rows out of 8
      C A
00001 3 4
00002 1 4

";

      var result = RunAndCaptureOutput(ARGS);

      Assert.AreEqual(EXPECTED_OUTPUT, result);
    }

I run the tests and now everything passes. Yay!

(This took way longer than I expected, although to be honest most of the time was spent on writing the blog article, not the code.)

The final code is available on GitHub. The classes look a bit better than in the previous attempt but not significantly so; the rewrite is probably not justified - but then again, it took less than a day even while writing this article at the same time, so it wasn't that much of a waste.

Well… that's it for now. Hope this helps.

Has to

One of the things I've come to hate when it comes to jobs is when the boss says "it has to be done by …". Whenever I hear that, I see red. Seriously. What does that even mean? The world will end if that task is not done in time? Global warming, zombie invasion, what?

As far as I can tell, what it actually means is "I / someone made a booboo and promised the customer that it will be done by … Can you save my / their ass?" Now, this makes sense. A mistake was made (not by me) and I am being asked to fix it. The mistake might actually be costly - the company might lose a hundred thousand dollars if the task is not done on time.

Which leads me to the second problem: why is it that, when someone else makes the mistake and I am supposed to fix it, I never get any of the benefits? Why is this problem never put as "I've made a mistake that is going to cost the company $100K unless you finish the job by Friday. However, if you do, I'll give you a $30K bonus"?

This has actually happened to me. I worked for a company that bought another one for its flagship product. Only, they forgot to pay attention to the fine print and discovered that the source code for the product was not included in the sale - they could run it but couldn't see / change what it did. (I mean, really, who does that?) Anyway, me and a colleague stayed at the office and wrote that product, from scratch, in two days. (We never left for home until it was finished.) We didn't even get a "good job", let alone a bonus.

Of course, if I "dare" to restate the problem this way, what usually happens is that I'm being told that my reward is keeping my job. Which is doubly stupid - firing me won't make the problem get solved any faster, so either you can't do it or the problem wasn't that big in the first place.

Anyway… I was reading Scott's list and one of the comments triggered this rant. As for the list itself, if I knew all that - as in "always have it in memory", not "knows how to look it up when needed" I would probably ask for a salary at Anders Hejlsberg's level. In fact, Atwood has a great response to that list: "Memorizing the answers to difficult technical questions won't save our jobs."

Thursday, June 13, 2013

Fail-over algorithm

oDesk and similar sites are an interesting source of programming ideas - there are a couple of job descriptions I've seen that I have found interesting. One of them went something like this (paraphrasing):

I need to continuously get data from a site and write it to a database. They have a main API that I coded for, but it fails sometimes. There is a secondary API and I coded for that too, but the first one fails unexpectedly and I haven't yet figured out all cases. Plus, the secondary one can fail too, I want to be informed if that happens.

Expressing this requirement in a different way, and assuming that the "want to be informed" part can also fail, the algorithm would look something like this:

  • try the first API
  • if that fails, log and try the second API
  • if that fails, log and inform the client
  • if that fails, log and write to an alert file
  • if that fails… do nothing, it's more important for the program to keep running

"Challenge accepted," as they say on the internet, even though the above way of putting the problem makes the solution embarrassingly easy.

First acceptance test

As usual, I start by writing an acceptance test. It should show a minimally functioning system — in my case, I want the first call to fail and the second one to succeed. This will allow me to verify both the fail-over mechanism and the logging. Here is the AcceptanceTests class:

  [TestClass]
  public class AcceptanceTests
  {
    [TestMethod]
    public void TestFailoverMechanism()
    {
      var stream = new MemoryStream();
      var logger = new StreamLogger(stream);
      logger.SystemClock = () => new DateTime(2000, 1, 1, 14, 15, 16);
      var guard = new FailGuard(logger);

      var success = false;
      guard.Attempt(
        () => { throw new Exception("Call failed."); },
        () => { success = true; });

      Assert.IsTrue(success);
      var lines = GetLines(stream);
      Assert.AreEqual("[2000.01.01 14:15:16 E] Call failed.", lines[0]);
      Assert.AreEqual("[2000.01.01 14:15:16 D] Success.", lines[1]);
    }

    //

    private static string[] GetLines(Stream stream)
    {
      stream.Seek(0, SeekOrigin.Begin);
      var reader = new StreamReader(stream);

      return reader
        .ReadToEnd()
        .Split(new[] { Environment.NewLine }, StringSplitOptions.None);
    }
  }

To explain the test: I want to test the system, not just the fail-over mechanism, which means I need to test the logger too. I want to verify that the logger writes the current date/time correctly, which means I need to force it to read a specific date/time. I could have just used DateTime.Now and hoped that the whole thing would take less than a second – which would be a reasonable assumption – but I think that's a bad habit to get into. The logger component has a dependency on the current date/time, that dependency should be exposed.

I will start by making the whole thing compile. Add the following to the library project, each interface / class in its own file of course. (I see that I haven't mentioned this — I split the solution into a library project called Failover.Library and a tests project called Failover.Tests; I have also added the Moq package, my favorite mocking library, to the tests project.)

  public interface Logger
  {
  }
  public class StreamLogger : Logger
  {
    public Func<DateTime> SystemClock = () => DateTime.Now;

    public StreamLogger(Stream stream)
    {
      //
    }
  }
  public class FailGuard
  {
    public FailGuard(Logger logger)
    {
      //
    }

    public void Attempt(params Action[] actions)
    {
      //
    }
  }

The project compiles and the test fails. All is as it should be.

First unit test

Not that I think the FailGuard class will be complicated, but the StreamLogger class should be really easy so that's what I'll start with. The acceptance test indicates that the StreamLogger class can log both regular messages and exceptions - they will have a "D" and an "E", respectively, in the line prefix part. I will start with the "D":

  [TestClass]
  public class StreamLoggerTests
  {
    [TestClass]
    public class Debug : StreamLoggerTests
    {
      [TestMethod]
      public void WritesTheGivenMessage()
      {
        const string MESSAGE = "abc123";

        var stream = new MemoryStream();
        var sut = new StreamLogger(stream);
        sut.SystemClock = () => new DateTime(2000, 1, 2, 3, 4, 5);

        sut.Debug(MESSAGE);

        var lines = GetLines(stream);
        Assert.IsTrue(lines[0].EndsWith(MESSAGE));
      }
    }

    //

    private static string[] GetLines(Stream stream)
    {
      stream.Seek(0, SeekOrigin.Begin);
      var reader = new StreamReader(stream);

      return reader
        .ReadToEnd()
        .Split(new[] { Environment.NewLine }, StringSplitOptions.None);
    }
  }

As you can see, I follow a number of conventions:

  • a test class for each class being tested, containing
  • a test class (inheriting from the one surrounding it) per method
  • the instance being tested is called sut, from "system under test"

To make this compile, the StreamLogger class needs a Debug method:

    public void Debug(string message)
    {
      //
    }

Everything compiles and the test fails. Good! Let's make it pass:

  public class StreamLogger : Logger
  {
    public Func<DateTime> SystemClock = () => DateTime.Now;

    public StreamLogger(Stream stream)
    {
      writer = new StreamWriter(stream);
    }

    public void Debug(string message)
    {
      writer.WriteLine(message);
    }

    //

    private readonly StreamWriter writer;
  }

The test… fails? (This stumped me for a few seconds.) Ah… I forgot; the streams need to be flushed in order for the write to actually happen. I could add a call to the Flush method after each write but that's error-prone. Instead, change the constructor of the StreamLogger class to:

    public StreamLogger(Stream stream)
    {
      writer = new StreamWriter(stream) { AutoFlush = true };
    }

The unit test passes. Great!

Another convention I'm using is that I put all the private members after the public ones, separated by an empty comment line. I just don't like the common convention that the first thing you see when looking at a class are the parts that are supposed to be hidden.

Duplication is bad

Ok… before rushing to do anything else, there's ugly duplication in the tests: the GetLines method. It needs to be moved to a helper class (add this to the test project, not the library):

  public static class Helper
  {
    public static string[] GetLines(this Stream stream)
    {
      stream.Seek(0, SeekOrigin.Begin);
      var reader = new StreamReader(stream);

      return reader
        .ReadToEnd()
        .Split(new[] { Environment.NewLine }, StringSplitOptions.None);
    }
  }

As you can see, the GetLines is an extension method, which means the tests change to:

  [TestClass]
  public class StreamLoggerTests
  {
    [TestClass]
    public class Debug : StreamLoggerTests
    {
      [TestMethod]
      public void WritesTheGivenMessage()
      {
        const string MESSAGE = "abc123";

        var stream = new MemoryStream();
        var sut = new StreamLogger(stream);
        sut.SystemClock = () => new DateTime(2000, 1, 2, 3, 4, 5);

        sut.Debug(MESSAGE);

        var lines = stream.GetLines();
        Assert.IsTrue(lines[0].EndsWith(MESSAGE));
      }
    }
  }

and

  [TestClass]
  public class AcceptanceTests
  {
    [TestMethod]
    public void TestFailoverMechanism()
    {
      var stream = new MemoryStream();
      var logger = new StreamLogger(stream);
      logger.SystemClock = () => new DateTime(2000, 1, 1, 14, 15, 16);
      var guard = new FailGuard(logger);

      var success = false;
      guard.Attempt(
        () => { throw new Exception("Call failed."); },
        () => { success = true; });

      Assert.IsTrue(success);
      var lines = stream.GetLines();
      Assert.AreEqual("[2000.01.01 14:15:16 E] Call failed.", lines[0]);
      Assert.AreEqual("[2000.01.01 14:15:16 D] Success.", lines[1]);
    }
  }

The unit test is still passing, the acceptance test still failing - all is good.

Second unit test

The Debug method needs to also write the current date/time; add a new test to the StreamLoggerTests.Debug class:


      [TestMethod]
      public void WritesTheCurrentDateTime()
      {
        var stream = new MemoryStream();
        var sut = new StreamLogger(stream);
        sut.SystemClock = () => new DateTime(2000, 1, 2, 3, 4, 5);

        sut.Debug("");

        var lines = stream.GetLines();
        Assert.IsTrue(lines[0].StartsWith("[2000.01.02 03:04:05"));
      }

(Since I don't care about the message in this test, I just sent an empty string.)

The test fails (as it should). To make it pass, change the StreamLogger.Debug method to:

    public void Debug(string message)
    {
      writer.WriteLine("[{0:yyyy.MM.dd hh:mm:ss}] {1}", SystemClock.Invoke(), message);
    }

The test passes. Almost there!

Duplication is really bad

There is some duplication in the StreamLoggerTests class. Let's get rid of it:

  [TestClass]
  public class StreamLoggerTests
  {
    [TestInitialize]
    public void SetUp()
    {
      stream = new MemoryStream();
      sut = new StreamLogger(stream) { SystemClock = () => new DateTime(2000, 1, 2, 3, 4, 5) };
    }

    [TestClass]
    public class Debug : StreamLoggerTests
    {
      [TestMethod]
      public void WritesTheGivenMessage()
      {
        const string MESSAGE = "abc123";

        sut.Debug(MESSAGE);

        var lines = stream.GetLines();
        Assert.IsTrue(lines[0].EndsWith(MESSAGE));
      }

      [TestMethod]
      public void WritesTheCurrentDateTime()
      {
        sut.Debug("");

        var lines = stream.GetLines();
        Assert.IsTrue(lines[0].StartsWith("[2000.01.02 03:04:05"));
      }
    }

    //

    private Stream stream;
    private StreamLogger sut;
  }

The tests are still passing. (Always check this - refactoring can be tricky and can create bugs.)

Final test for the Debug method

The final test for the Debug method is to verify that the "D" gets written:

      [TestMethod]
      public void Writes_D_AtTheEndOfThePrefix()
      {
        sut.Debug("");

        var lines = stream.GetLines();
        Assert.AreEqual("D]", lines[0].Substring(21, 2));
      }

The test fails. Making it pass is trivial:

    public void Debug(string message)
    {
      writer.WriteLine("[{0:yyyy.MM.dd hh:mm:ss} D] {1}", SystemClock.Invoke(), message);
    }

Nothing to refactor. The code is extremely simple and it works correctly.

Logging errors

Add the following inside the StreamLoggerTests class:

    [TestClass]
    public class Error: StreamLoggerTests
    {
      [TestMethod]
      public void WritesTheGivenMessage()
      {
        const string MESSAGE = "abc123";

        sut.Error(new Exception(MESSAGE));

        var lines = stream.GetLines();
        Assert.IsTrue(lines[0].EndsWith(MESSAGE));
      }
    }

This needs a new method in the StreamLogger class:

    public void Error(Exception exception)
    {
      //
    }

The test fails; to make it pass, change this method to:

    public void Error(Exception exception)
    {
      writer.WriteLine(exception.Message);
    }

(Note that I do not rush to implement the entire functionality, even though I know what I'm supposed to end with. It is better to proceed in small steps and make sure everything works before starting on the next one.)

The second test is almost identical to the one for the Debug method:

      [TestMethod]
      public void WritesTheCurrentDateTime()
      {
        sut.Error(new Exception(""));

        var lines = stream.GetLines();
        Assert.IsTrue(lines[0].StartsWith("[2000.01.02 03:04:05"));
      }

It fails, so I change the method to:

    public void Error(Exception exception)
    {
      writer.WriteLine("[{0:yyyy.MM.dd hh:mm:ss}] {1}", SystemClock.Invoke(), exception.Message);
    }

Finally, the third test:

      [TestMethod]
      public void Writes_E_AtTheEndOfThePrefix()
      {
        sut.Error(new Exception(""));

        var lines = stream.GetLines();
        Assert.AreEqual("E]", lines[0].Substring(21, 2));
      }

and the matching code change (but only after verifying that the test fails):

    public void Error(Exception exception)
    {
      writer.WriteLine("[{0:yyyy.MM.dd hh:mm:ss} E] {1}", SystemClock.Invoke(), exception.Message);
    }

Duplication, duplication, duplication

The StreamLogger class is begging for a refactoring:

  public class StreamLogger : Logger
  {
    public Func<DateTime> SystemClock = () => DateTime.Now;

    public StreamLogger(Stream stream)
    {
      writer = new StreamWriter(stream) { AutoFlush = true };
    }

    public void Debug(string message)
    {
      WriteMessage("D", message);
    }

    public void Error(Exception exception)
    {
      WriteMessage("E", exception.Message);
    }

    //

    private readonly StreamWriter writer;

    private void WriteMessage(string messageType, string message)
    {
      writer.WriteLine("[{0:yyyy.MM.dd hh:mm:ss} {1}] {2}", SystemClock.Invoke(), messageType, message);
    }
  }

The tests pass; the code is crystal clear; all is good. Whew!

The FailGuard class

We're now getting to the main reason for this article: the FailGuard class. Let's write a test to verify that the Attempt method runs a single action:

  [TestClass]
  public class FailGuardTests
  {
    [TestClass]
    public class Attempt : FailGuardTests
    {
      [TestMethod]
      public void RunsSingleAction()
      {
        var logger = new Mock<Logger>();
        var sut = new FailGuard(logger.Object);
        var success = false;

        sut.Attempt(() => { success = true; });

        Assert.IsTrue(success);
      }
    }
  }

As before, the encompassing FailGuardTests class contains an Attempt class, which in turn contains the first test — namely the RunsSingleAction method. The test fails, so to make it pass change the FailGuard.Attempt method to:

    public void Attempt(params Action[] actions)
    {
      actions[0].Invoke();
    }

This immediately suggests another test, which I will write after verifying that the first one passes — what happens if there are no actions?

      [TestMethod]
      public void DoesNothingIfThereAreNoActions()
      {
        var logger = new Mock<Logger>();
        var sut = new FailGuard(logger.Object);

        sut.Attempt();

        // success - did not crash
      }

The test crashes, as expected, so I'll change the production code to:

    public void Attempt(params Action[] actions)
    {
      if (!actions.Any())
        return;

      actions[0].Invoke();
    }

Both unit tests pass; time to refactor:

  [TestClass]
  public class FailGuardTests
  {
    [TestInitialize]
    public void SetUp()
    {
      logger = new Mock<Logger>();
      sut = new FailGuard(logger.Object);
    }

    [TestClass]
    public class Attempt : FailGuardTests
    {
      [TestMethod]
      public void RunsSingleAction()
      {
        var success = false;

        sut.Attempt(() => { success = true; });

        Assert.IsTrue(success);
      }

      [TestMethod]
      public void DoesNothingIfThereAreNoActions()
      {
        sut.Attempt();

        // success - did not crash
      }
    }

    //

    private Mock<Logger> logger;
    private FailGuard sut;
  }

All good.

Fail-over mechanism

We've finally reached the intended functionality: the fail-over mechanism. Let's write a unit test for that:

      [TestMethod]
      public void ContinuesToSecondActionIfTheFirstOneFails()
      {
        var success = false;

        sut.Attempt(
          () => { throw new Exception(); },
          () => { success = true; });

        Assert.IsTrue(success);
      }

This fails - the Attempt method does not guard against the exception. Let's fix that:

    public void Attempt(params Action[] actions)
    {
      if (!actions.Any())
        return;

      try
      {
        actions[0].Invoke();
      }
      catch (Exception)
      {
        actions[1].Invoke();
      }
    }

Note that I'm always doing "the simplest thing that could possibly work". Yes, I could go ahead and write the whole logic but then I wouldn't have any tests to guard against bugs — and if I tried to add them after the code they would tend to use the knowledge I have about the implementation. It can work, but it's dangerous. Let's stick to the TDD method.

Second note: the acceptance test is now failing in a different place. Progress!

Of course, now that I've made the test pass I can guard against a problem: what happens if the (only) action fails? I'll make a note to come back to that later, but I've neglected logging and I want to take care of it first.

Logging

Start with the logging for the "single action working" case:

      [TestMethod]
      public void LogsSuccessfulSingleAction()
      {
        sut.Attempt(() => { });

        logger.Verify(it => it.Debug("Success."));
      }

This… does not compile, because I forgot to add the Debug and Error methods to the Logger interface. Oops:

  public interface Logger
  {
    void Debug(string message);
    void Error(Exception exception);
  }

Better. The test fails, so I'm changing the FailGuard class to:

  public class FailGuard
  {
    public FailGuard(Logger logger)
    {
      this.logger = logger;
    }

    public void Attempt(params Action[] actions)
    {
      if (!actions.Any())
        return;

      try
      {
        actions[0].Invoke();
        logger.Debug("Success.");
      }
      catch (Exception)
      {
        actions[1].Invoke();
      }
    }

    //

    private readonly Logger logger;
  }

Yay!

Of course, the logger needs to be informed of errors too:

      [TestMethod]
      public void LogsError()
      {
        const string MESSAGE = "abc";

        sut.Attempt(
          () => { throw new Exception(MESSAGE); },
          () => { });

        logger.Verify(it => it.Error(It.Is<Exception>(e => e.Message == MESSAGE)));
      }

To make it pass, I change the production code to:

    public void Attempt(params Action[] actions)
    {
      if (!actions.Any())
        return;

      try
      {
        actions[0].Invoke();
        logger.Debug("Success.");
      }
      catch (Exception ex)
      {
        logger.Error(ex);
        actions[1].Invoke();
      }
    }

That passes. Unfortunately, the acceptance test now breaks in an interesting way:

Assert.AreEqual failed. Expected:<14:15:16 E Call failed.>. Actual:<02:15:16 E Call failed.>.

Oops again? I've been bitten by the time formatting string specifier. Easy to fix by changing the StreamLogger.WriteMessage method to:

    private void WriteMessage(string messageType, string message)
    {
      writer.WriteLine("[{0:yyyy.MM.dd HH:mm:ss} {1}] {2}", SystemClock.Invoke(), messageType, message);
    }

Good.

Fixing the algorithm

You will note that my last test (the FailGuardTests.LogsError method) has an additional empty action. That's because without it the Attempt method would fail - it tries to access an inexistent second action.

I'll modify the LogsError test to show the problem:

      [TestMethod]
      public void LogsError()
      {
        const string MESSAGE = "abc";

        sut.Attempt(() => { throw new Exception(MESSAGE); });

        logger.Verify(it => it.Error(It.Is<Exception>(e => e.Message == MESSAGE)));
      }

Fixing it is again easy:

    public void Attempt(params Action[] actions)
    {
      if (!actions.Any())
        return;

      try
      {
        actions[0].Invoke();
        logger.Debug("Success.");
      }
      catch (Exception ex)
      {
        logger.Error(ex);

        if (actions.Length > 1)
          actions[1].Invoke();
      }
    }

Of course, this is starting to become ugly. I need a test with several actions. In fact, I am going to write two tests - one where the last action succeeds and one where it fails; both of them will check the logs. I will start with the first one:

      [TestMethod]
      public void LogsErrorsAndThenSuccess()
      {
        var sequence = new MockSequence();
        logger.InSequence(sequence).Setup(it => it.Error((It.Is<Exception>(e => e.Message == "1")))).Verifiable();
        logger.InSequence(sequence).Setup(it => it.Error((It.Is<Exception>(e => e.Message == "2")))).Verifiable();
        logger.InSequence(sequence).Setup(it => it.Error((It.Is<Exception>(e => e.Message == "3")))).Verifiable();
        logger.InSequence(sequence).Setup(it => it.Debug("Success")).Verifiable();
        
        sut.Attempt(
          () => { throw new Exception("1"); },
          () => { throw new Exception("2"); },
          () => { throw new Exception("3"); },
          () => { });

        logger.Verify();
      }

This fails, which forces me to write the proper code in the Attempt method. (Note that "the simplest thing that could possibly work" now is not to keep adding ifs and special cases for the first 3 or 4 actions.)

    public void Attempt(params Action[] actions)
    {
      foreach (var action in actions)
      {
        try
        {
          action.Invoke();
          logger.Debug("Success.");
          break;
        }
        catch (Exception ex)
        {
          logger.Error(ex);
        }
      }
    }

(I have removed the initial Any() call because it was no longer needed - the foreach will just not run if there are no actions.)

Not only is this making the unit test pass, but the acceptance test too! We're basically done, except I want to make sure and I write the final unit test, where all the actions fail:

      [TestMethod]
      public void LogsOnlyErrors()
      {
        var sequence = new MockSequence();
        logger.InSequence(sequence).Setup(it => it.Error((It.Is<Exception>(e => e.Message == "1")))).Verifiable();
        logger.InSequence(sequence).Setup(it => it.Error((It.Is<Exception>(e => e.Message == "2")))).Verifiable();
        logger.InSequence(sequence).Setup(it => it.Error((It.Is<Exception>(e => e.Message == "3")))).Verifiable();

        sut.Attempt(
          () => { throw new Exception("1"); },
          () => { throw new Exception("2"); },
          () => { throw new Exception("3"); });

        logger.Verify();
      }

To paraphrase a common proverb, this too has passed. We're done!

Conclusion

The production classes (and interface) look like this:

  public class FailGuard
  {
    public FailGuard(Logger logger)
    {
      this.logger = logger;
    }

    public void Attempt(params Action[] actions)
    {
      foreach (var action in actions)
      {
        try
        {
          action.Invoke();
          logger.Debug("Success.");
          break;
        }
        catch (Exception ex)
        {
          logger.Error(ex);
        }
      }
    }

    //

    private readonly Logger logger;
  }
  public interface Logger
  {
    void Debug(string message);
    void Error(Exception exception);
  }
  public class StreamLogger : Logger
  {
    public Func<DateTime> SystemClock = () => DateTime.Now;

    public StreamLogger(Stream stream)
    {
      writer = new StreamWriter(stream) { AutoFlush = true };
    }

    public void Debug(string message)
    {
      WriteMessage("D", message);
    }

    public void Error(Exception exception)
    {
      WriteMessage("E", exception.Message);
    }

    //

    private readonly StreamWriter writer;

    private void WriteMessage(string messageType, string message)
    {
      writer.WriteLine("[{0:yyyy.MM.dd HH:mm:ss} {1}] {2}", SystemClock.Invoke(), messageType, message);
    }
  }

You might say that they are too simple. Agreed… except, I have seen production code with simpler requirements that was five times their size and an incredibly convoluted mess. These classes are fairly reusable and easy to understand and maintain — and we have the tests to prove that they work as expected. Not bad for a couple of hours of coding.

Edit: The source code is available on GitHub.