L'Agilitateur Création de logiciels : de l'agilité à l'artisanat

James Shore proposes to experiment a design challenge to deal with architecture issue. It was posted a week ago and some answers already came out. I haven't looked at them yet and I wanted to try on my own. So, here it is.
My solution is coded in C# with basic tooling : Monodevelop IDE, NUnit testing framework and Moq mocking framework.

The main logic of the software to build deals about ROT13. So let's start with a basic test for ROT13 on individual chars.

public void PerformRot13OnIndividualChars ()
{
  Assert.That( Rot13Converter.CharRot13('A'), Is.EqualTo('N') );
  Assert.That( Rot13Converter.CharRot13('U'), Is.EqualTo('H') );
  Assert.That( Rot13Converter.CharRot13('c'), Is.EqualTo('p') );
  Assert.That( Rot13Converter.CharRot13('s'), Is.EqualTo('f') );
  Assert.That( Rot13Converter.CharRot13('3'), Is.EqualTo('3') );
  Assert.That( Rot13Converter.CharRot13('*'), Is.EqualTo('*') );
}

And then we come with a simple implementation of the char rotation method. It might be improved for non-English chars but these details are not relevant for our problem.

public class Rot13Converter
{
  public static char CharRot13(char c)
  {
    if( char.IsUpper(c) )
      return CharRot13Impl(c,'A');
    else if( char.IsLower(c) )
      return CharRot13Impl(c,'a');
    else
      return c;
  }
	
  static char CharRot13Impl(char inputChar, char baseChar)
  {
    return Convert.ToChar(AsciiRot13(Convert.ToInt32(inputChar),Convert.ToInt32(baseChar)));
  }
	
  static int AsciiRot13(int inputCode, int baseCode)
  {
    return ((inputCode - baseCode + 13) % 26) + baseCode;
  }
}

Now it's time to design the conversion logic as a whole. Since we want to unit test the logic with no I/O code, we'll have to mock the I/O layer.
Beside this, since Uncle Bob once said ''High-level modules should not depend on low-level modules. Both should depend on abstractions'', we all know that a good approach is to create an abstraction of the I/O layer and let the logic depend on it.

We're dealing with a conversion of a char stream into another char stream. So a char stream might be a good abstraction.
But, wait a minute. Uncle Bob also said ''Clients should not be forced to depend upon interfaces that they do not use''. If my abstraction is a char stream, then it will probably contains ways to read the stream and to write into the stream, which means that, for an I/O implementation where we only have to write to (e.g. a console), we will bring unnecessary dependency to the reading part of the abstraction.

As a consequence, we will prefer 2 abstractions : an input char stream and an output char stream. Now we can code a unit test for the ROT13 logic on char streams.

public void TransformCharStreamIntoRot13edCharStream ()
{
  var inputStream = new Mock<InputCharStream>();
  inputStream
    .Setup( stream => stream.HasChars() )
    .ReturnsInOrder( true, true, false );
  inputStream
    .Setup( stream => stream.GetChar() )
    .ReturnsInOrder( 'A', 'c' );

  var outputStream = new Mock<OutputCharStream>();      
  var converter = new Rot13Converter();

  converter.ExecuteOn( inputStream.Object, outputStream.Object );

  outputStream.Verify( stream => stream.PutChar('N'), Times.Once() );
  outputStream.Verify( stream => stream.PutChar('p'), Times.Once() );
  outputStream.Verify( stream => stream.PutChar(It.IsAny<char>()), Times.Exactly(2) );
}

And then we write the code to pass the test.

public interface InputCharStream
{
  char GetChar();
  bool HasChars();
}

public interface OutputCharStream
{
  void PutChar(char c);
}

public class Rot13Converter
{
  public void ExecuteOn (InputCharStream inputStream,  OutputCharStream outputStream)
  {
    while( inputStream.HasChars() )
      outputStream.PutChar( CharRot13(inputStream.GetChar()) );
  }
}

We're done with the logic !

Next step : coding the I/O, with focused integration tests.

First we are going to deal with the part where we read data from an input char stream implemented as a file on disk. We create a text input file and we write a test.

public void ReadFileFromDisk()
{
  var reader = new FileReader( "someInput.txt", new TestConfiguration() );
  Assert.That( reader.HasChars(), Is.True );
  Assert.That( reader.GetChar(), Is.EqualTo('A') );
  Assert.That( reader.HasChars(), Is.True );
  Assert.That( reader.GetChar(), Is.EqualTo('B') );
  Assert.That( reader.HasChars(), Is.True );
  Assert.That( reader.GetChar(), Is.EqualTo('C') );
  Assert.That( reader.HasChars(), Is.False );
}

A key decision is how to design the configuration part. We write a toy program where file on disk is there to mimic some data storage, so the configuration might be anything : a folder on disk, a server name, etc.
Since the FileReader might need multiple information from the configuration, we will use dependency inversion again : we define a configuration as an abstraction and give an implementation of this abstraction to the FileReader.
So, basically, we define a FileReader by giving a storage context (the configuration) and the data we want to read (here, a file name).
We write the code to pass the test and that's it.

public interface Configuration
{
  string GetDataPath (string fileName);
}

public class FileReader : InputCharStream
{
  char[] content;
  int index;

  public FileReader(string fileName, Configuration configuration)
  {
    content = System.IO.File.ReadAllText(configuration.GetDataPath(fileName)).ToCharArray();
    index = 0;
  }

  public char GetChar ()
  {
    return content[index++];
  }

  public bool HasChars ()
  {
    return index < content.Length;
  }
}

As requested, we implemented a solution where the data is first read into the memory as a whole.

Now we are going to implement an output char stream to write our ROT13 data. We need to write it to console output and to file on disk. We could bundle both writes in a single class but that would not really follow the Single Responsibility Principle. So we will write 2 classes : one for writing to file on disk and the other one to write to console output.
Actually we need a third one. Since our logic code writes into a single output stream, we will also create an output duplicator : a class that implements the OutputCharStream and that duplicates the output on multiple output streams.

We will start with this output duplicator. For that, we leave the world of I/O and go back to our unit tests.

public void DuplicateCharStream()
{
  var outputStream1 = new Mock<OutputCharStream>();      
  var outputStream2 = new Mock<OutputCharStream>(); 
  var multipleOutput = new MultiOutputCharStream( outputStream1.Object, outputStream2.Object );
  multipleOutput.PutChar('A');
  outputStream1.Verify(stream => stream.PutChar('A'));
  outputStream2.Verify(stream => stream.PutChar('A'));
  multipleOutput.PutChar('B');
  outputStream1.Verify(stream => stream.PutChar('B'));
  outputStream2.Verify(stream => stream.PutChar('B'));
}

We write the code to pass the test.

public class MultiOutputCharStream : OutputCharStream
{
  OutputCharStream[] streams;

  public MultiOutputCharStream(params OutputCharStream[] outputStreams)
  {
    streams = outputStreams;
  }

  public void PutChar(char c)
  {
    foreach(var stream in streams)
      stream.PutChar(c);
  }
}

Now we can go back to I/O and write a test for the stream output into a file on disk.

public void WriteFileToDisk()
{
  var config = new TestConfiguration();
  var fileName = "someOutput.txt";
  var filePath = config.GetDataPath(fileName);

  System.IO.File.Delete(filePath);
  Assert.That( System.IO.File.Exists(filePath), Is.False );

  var writer = new FileWriter( fileName, config );
  writer.PutChar('X');
  writer.PutChar('Y');
  writer.PutChar('Z');
  Assert.That( System.IO.File.Exists(filePath), Is.True );
  Assert.That( System.IO.File.ReadAllText(filePath), Is.EqualTo("XYZ") );
}

And we write the corresponding code.

public class FileWriter : OutputCharStream
{
  string filePath;

  public FileWriter(string fileName, Configuration configuration)
  {
    filePath = configuration.GetDataPath(fileName);
  }

  public void PutChar(char c)
  {
    System.IO.File.AppendAllText(filePath, c.ToString());
  }
}

We also need an output stream for the console but this one is hard to test automatically - as is every user interface test,
So we will code it directly. Anyway, a manual test would be very simple to write if needed.

public class ConsoleWriter : OutputCharStream
{
  public void PutChar(char c)
  {
    Console.Write(c);
  }
}

Now we're done with the I/O. We almost have all our parts, we just need a real configuration context and then we can link all these little classes in our main program.
We could use a dependency injection framework to do the job, but, since this program is quite simple we can just write all the code in our main routine :

class MainClass
{
  public static void Main (string[] args)
  {
    var config = new CurrentFolderConfiguration();
    var converter = new Rot13Converter();
    converter.ExecuteOn(
      new FileReader(args[0], config),
      new MultiOutputCharStream(
        new FileWriter(args[1], config),
        new ConsoleWriter()
      )
    );
    Console.WriteLine();
  }
}

And then we're done with Part I of the challenge!

Now, what do we have to change for part II? The logic is there, the config mechanism too. The output part does not change. We just need to change our FileReader so that it does not load everything into memory and see if we have an impact on the other classes.
We know that ''Classes should be open for extension but closed for modification'' and our design came naturally with extension facilities. So we will not modify our File Reader : we will create a LazyFileReader that will do the same job but loading chars only when they are needed.

The test for this LazyFileReader is quite similar to the FileReader but it is better to have separate tests so that we can add any specific assertions if needed.

public void ReadLazilyFileFromDisk()
{
  var reader = new LazyFileReader( "someInput.txt", new TestConfiguration() );
  Assert.That( reader.HasChars(), Is.True );
  Assert.That( reader.GetChar(), Is.EqualTo('A') );
  Assert.That( reader.HasChars(), Is.True );
  Assert.That( reader.GetChar(), Is.EqualTo('B') );
  Assert.That( reader.HasChars(), Is.True );
  Assert.That( reader.GetChar(), Is.EqualTo('C') );
  Assert.That( reader.HasChars(), Is.False );
}

And then we have the code of the LazyFileReader

public class LazyFileReader : InputCharStream
{
  System.IO.StreamReader readerImpl;

  public LazyFileReader(string fileName, Configuration configuration)
  {
    readerImpl = System.IO.File.OpenText(configuration.GetDataPath(fileName));
  }

  public char GetChar()
  {
    return Convert.ToChar(readerImpl.Read());
  }

  public bool HasChars()
  {
    return !readerImpl.EndOfStream;
  }
}

In order to make our program use the lazy reader, we just have to change the reader creation in our main program :

class MainClass
{
  public static void Main (string[] args)
  {
    var config = new CurrentFolderConfiguration();
    var converter = new Rot13Converter();
    converter.ExecuteOn(
      new LazyFileReader(args[0], config),
      new MultiOutputCharStream(
        new FileWriter(args[1], config),
        new ConsoleWriter()
      )
    );
    Console.WriteLine();
  }
}

Note that if we had used a dependency injection framework, we could have done this change in the main program through a simple configuration file which means no code change at all : we would only have added a new class to the code base!

Now I think it's time to draw conclusions but I will first review the proposed solutions to see if I have correctly understood the challenge. I saw that James Shore published today a new blog entry on this topic.
Up to now, all I can say is that this challenge made me review the S.O.L.I.D. principles -which is always good- but did not led me to think about big software architecture issues.

Note : the full source code and project files are attached to this blog entry.
Note 2 : the source code is now also available on github : http://github.com/Oaz/JamesShoreArchitecturalDesignChallenge

Addition June 6th : class diagram (click on it for bigger size)