Thursday, 7 January 2010

Where did I go wrong in TDD Interview?

I failed a technical interview where I had to prove that I understood TDD.

Can an expert TDD person show me where I went wrong?

Please be as brutally honest as possible. I am keen to learn from whatever mistake(s) I have made.

I just failed a job interview which wanted people skilled in TDD . I was given the following task.

Write code that will;

1) Read an input file.
2) Reverse the contents.
3) Write the reversed contents to an output file.

Example:
Input file contains “Company”
After processing the input file, an output file should be written containing "ynapmoC"

Here is one of the main test that I created using TDD.

I think the main thing to point out is that I isolated the actual file read/write functionality by wrapping it in my own interface. So the test does not actually read or write files. (I had other tests checking for empty strings, nulls, and non existent files.)

[TestMethod]
public void ThreeCharactersShouldBeReversed()
{
Mockery mockery = new Mockery();
IReadWriter readWriter = (IReadWriter)mockery.NewMock(typeof(IReadWriter));

Expect.Once.On(readWriter).Method("ReadStreamContent").Will(Return.Value("XYZ"));
Expect.Once.On(readWriter).Method("WriteStreamContent").With("ZYX");

IReverser reverser = new ContentReverser(readWriter);

bool result = reverser.ReverseFileContents();

Assert.IsTrue(result, "Expected true to be returned indicating success");

mockery.VerifyAllExpectationsHaveBeenMet();
}

Here’s the reverser class.

public interface IReverser
{
bool ReverseFileContents();
}

public class ContentReverser : IReverser
{
private readonly IReadWriter _readWriter;
public ContentReverser(IReadWriter readWriter)
{
_readWriter = readWriter;
}

public bool ReverseFileContents()
{
bool result;
try
{
string sourceContent = _readWriter.ReadStreamContent();

char[] originalContent = sourceContent.ToCharArray();
Array.Reverse(originalContent);
string reversedContent = new string(originalContent);

_readWriter.WriteStreamContent(reversedContent);

result = true;
}
catch (Exception)
{
result = false;
}

return result;
}
}

Here’s a class that actually does read/writing of files. (Not used by the above Test, Although during the interview I wrote a UI that instantiated and passed in one of these classes to demonstrate integration testing)

public interface IReadWriter
{
string ReadStreamContent();
void WriteStreamContent(string fileContent);
}

/// <summary>
/// This is the physical file read and write mechanism.
/// </summary>
public class ReadWriter : IReadWriter
{
private readonly string _sourceFile;
private readonly string _targetFile;

public ReadWriter(string sourceFile, string targetFile)
{
_sourceFile = sourceFile;
_targetFile = targetFile;
}

public string ReadStreamContent()
{
return System.IO.File.ReadAllText(_sourceFile);
}

public void WriteStreamContent(string fileContent)
{
System.IO.File.WriteAllText(_targetFile, fileContent);
}
}

8 comments:

  1. are you using nmock? try some rhino

    readWriter.Expect(rw => rw.ReadStreamContent()).Return("XYZ").Repeat.Once;
    readWriter.Expect(rw => rw.WriteStreamContent("XYZ")).Repeat.Once;

    doubt they'd not hire you because of that tho. maybe you weren't test-driven enough, did you start out with a very simple test and forumate teh design from there?

    ReplyDelete
  2. Thanks for the comment. Yes I started with a very simple test case. Yeah I've been looking at Rhino, think I will switch, it seems to be more popular.

    ReplyDelete
  3. Good post. I'll not talk about the implementation, but focus on the tests. I'm going to assume that the test you have used is an exact copy from your tests, and you haven't added more code into the test method just so you didn't have to show the whole test class.

    I am a big fan of keeping test methods as simple and as literate as possible. This has the great side effect of decoupling your tests from the production code, and therefore makes it easier to refactor your production code without spending ages keeping tests up to date. For me, the lightbuld went on after reading Bob Martins 'Clean Code' book.

    Here is how I would write the test you have given above. I've also added a few additional tests to show how effective this approach can be.

    It's a bit of a mish mash of Java (because that's what I work in mostly, and C#, your example). It obviously won't run in either language:)

    public class ContentReverserTest {
    Mockery mockery;
    IReadWriter readWriter;

    @Before
    public void init() {
    mockery = new Mockery();
    readWriter = (IReadWriter)mockery.NewMock(typeof(IReadWriter));
    }

    @Test
    public void threeCharactersShouldBeReversed() {
    whenReading("XYZ");
    expectWriteAs("ZYX");
    }

    @Test
    public void emptyStringIsHandled() {
    whenReading("");
    expectWriteAs("");
    }

    @Test
    public void multipleLinesCanBeReversed() {
    whenReading("new lines\nare handled\nfine too");
    expectWriteAs("oot enif\ndeldnah eran\nsenil wen");
    }

    @Test
    public void utf8CharactersCanBeReversed() {
    whenReading("ก 丂 ب ç");
    expectWriteAs("ç ب 丂 ก");
    }

    private void whenReading(String s) {
    Expect.Once.On(readWriter).Method("ReadStreamContent").Will(Return.Value(s));
    }

    private void expectWriteAs(String s) {
    Expect.Once.On(readWriter).Method("WriteStreamContent").With(s);
    IReverser reverser = new ContentReverser(readWriter);
    bool result = reverser.ReverseFileContents();
    Assert.IsTrue(result, "Expected true to be returned indicating success");
    mockery.VerifyAllExpectationsHaveBeenMet();
    }
    }

    Unfortunately it doesn't look like I can format the code to be properly indented so it's a little tricky to read.

    As it happens I have written a blog entry about my light bulb moment. You might be interested in reading it: http://blog.jonmdickinson.com/2009/06/insulating-tests-from-interface-changes.html

    ReplyDelete
  4. Have you asked them for feedback on your tdd?

    ReplyDelete
  5. Jon, thank you very much for that. On other stuff I've done I've always suspected that my tests were not as simple as they could be. I think you have identified something that I need to improve.

    I am just about to read your blog. Once again thanks.

    ReplyDelete
  6. Robvdlv, I might try to do that. Trouble is I have not got direct contact with the company as I went through an agent. Think I will have a go though.

    ReplyDelete
  7. Another thing TDDers love is to learn what's gone wrong when reading the error message of a failing test.

    Also, why dont you test the string reversing separately from file io? They say tests should have only one reason to fail.

    ReplyDelete