test driven development 1

[dinger2121]

Hello,
I am trying to get up and running with tdd in c#, and have a question.
I am planning to create a class for processing a text file. This class will take in a file name, process the data, add it to an List, and return the list.

My question is this - what do I want to create tests for? I was thinking that I will only want to create a test to ensure that I am able to construct this object, and then one to ensure that a List is successfully returned. Is that correct?
I am kind of hung up on the correct approach to take and I'm not sure if I'm thinking about this the right way.

Thank for any thoughts.

carl
MCSD, MCTS:MOSS

 

[dinger2121]

Sorry - I think I was going in the wrong direction with this. I had my class constructed incorrectly, therefore I was not able to wrap my head around how to create test cases for it. I am going to rebuild my class and work through it again - hopefully that will clear some things up.
Thanks

carl
MCSD, MCTS:MOSS

 

[jmeckley]

One of the driving forces behind TDD is the test dictate the design of the class under test. Therefore you write the test first, then you write the code which makes the test pass.

creating the class and then writing tests against the class is not TDD. It still validates the class, but it isn't TDD by definition.



Jason Meckley
Programmer
Specialty Bakers, Inc.

faq855-7190
faq732-7259

 

[dinger2121]

But then how do you know what test to create? Is that driven by the requirements.
For instance, I want to create a class that reads in a text file and then returns a list of strings that it reads from the file.
I know that I need to a test to ensure that it is returning what I need, but how do I know what else to test for?

carl
MCSD, MCTS:MOSS

 

[jmeckley]

yes, the requirements drive what tests are needed, which in turn drives the design of the class.

so for your scenario you would need a test that accepts a textreader (super class) and returns a collection of strings. Now how many tests you need can vary. I would approach it this way.
1. your system will read a file, but testing against IO is slow and error prone. therefore i would look for a higher level abstraction then File. I would take a TextReader as the input. then you can use a StringReader for the test and a FileStream in production.
2. once i get the lines from the file I probably do not want to directly modify the collection. therefore i would return an IEnumerable<string>. if I need to manipulate the collection i can load it into a new list.
3. now if this call takes a TextReader I will another class to create the FileStream. This class may take a filename as the input and return a TextReader.
1st psuedo test

var reader = new StringReader(@"line1
line2");
var sut = new ReaderToStringCoverter()
var result = sut.Prase(reader);
Assert.That(result, Only.Contains("line1" ,"line2"));

and the implementation

class ReaderToStringCoverter
{
   public IEnumreable<string> Parse(TextReader reader)
   {
       while(var line = reader.ReadLine() != null)
       {
            yield return line;
       }
       
   }
}

then for the creation of a file steam

//setup
create file on disk

var sut = new FileStreamFactory();
var result = sut.CreateStreamFrom(nameOfFile);
Assert.That(result, Is.TypeOf<FileStream>());

and the code to make the test pass

class FileStreamFactory
{
   public TextReader CreateStreamFrom(string filename)
   {
      return new FileStream(filename);
   }
}

Jason Meckley
Programmer
Specialty Bakers, Inc.

faq855-7190
faq732-7259

 

[dinger2121]

I actually was able to get it working, but differently than yours. I was hoping you might take a look at mine and let me know where my implementation could be improved. Here is my implementation -

public class ImportFile : IImportFile
    {
        public ImportFile()
        {
            this.FileName = "";
        }

        #region IImportFile Members
        public string FileName { get; set; }

        public IList<string> GetFileData
        {
            get 
            {
                return _getFileData(); 
            }
        }

        #endregion

        private IList<string> _getFileData()
        {
            StreamReader reader = new StreamReader(this.FileName);
            IList<string> result = new List<string>();
            while (reader.Peek() >= 0)
            {
                result.Add(reader.ReadLine());
            }
            reader.Close();
            return result;
        }
    }

And here are my tests -

[Test]
        public void ImportFileConstuctorTest()
        {
            IImportFile import = new ImportFile();

            Assert.IsTrue(null != import, "Construction Failed");
            Assert.IsTrue("" == import.FileName,"Value should initially be blank");
        }

        [Test]
        public void FileNameTest()
        {
            ImportFile import = new ImportFile();
            import.FileName = "testFile";

            Assert.IsTrue(import.FileName == "testFile","Expected: testFile; actual: " + import.FileName);
        }

        [Test]
        public void GetFileDataTest()
        {
            ImportFile import = new ImportFile();
            import.FileName = "c:\\List_Test.txt";

            IList<string> expected = new List<String>()
            {
                "line one",
                "line two"
            };
            IList<string> actual = import.GetFileData;
            
            CollectionAssert.AreEqual(expected,actual,"The correct list data is not being pulled in");

        }

As always, thanks a lot for your help.

carl
MCSD, MCTS:MOSS

 

[jmeckley]

there is one change that should be implemented and that's disposing of the stream in _getFileData(). if any exception is thrown before you are finished reading the stream will not close. that can be cleaned up easily enough

List<string> result = new List<string>();
using(StreamReader reader = new StreamReader(this.FileName))
{
   while (reader.Peek() >= 0)
   {
      result.Add(reader.ReadLine());
   }
}
return result;

after that it's just preferences.
I try never to expose mutable lists publicly. therefore i would return an IEnumerable<string> rather than IList<string>.
If you are going to set the file name as a property then you should create an explicit exception to throw if the filename is not set. This error will be more meaningful when it's logged.

if (string.IsNullOrEmpty(FileName))
{
   throw new ImportFileNotConfiguredExecption();
}
//return list of strings from the file

class ImportFileNotConfiguredExecption : Exception
{
   public ImportFileNotConfiguredExecption()
      :base("No file specified to import. Set the FileName property before calling GetFileData");
}

I would also convert GetFileData to a method. that's strictly a coding preference.

other stylistic recommendations.
* The class has 2 responsiblities 1. access the file 2. return rows. consider separating them into 2 distinct classes. abstracting IO calls (file, socket, database, etc.) makes testing much easier.
* your test requires the artifact [tt]c:\\List_Test.txt[/tt]. I would create the file before the test and delete the file after the test. It's considered bad practice not to clean up after each test. I would also keep it within the executable directory rather than the root of C:
* rename the class to reveal the intent of what it does. Technically this class does not import data. It reads lines from a file. a name like FileLineReader or something would be more appropriate.
* You can pass in any text as a filename. the file may not exist, or it may not be a text file. in which case your code fails. by having the class use a TextReader you ensure the reader can process lines of text.

with the last point about checking the file and if it exists. if you keep this all in one place the class begins to build up too much responsibility for a single object. this also makes it more difficult to test because there are more scenarios you must account for to ensure you code works properly. by moving the creation of the text reader to another object reading lines from a file becomes a very simple test. and the object which creates the filestream is also easier to test because all you need to validate is 1. the filename is not null 2. the file exists and 3.a FileStream is returned.

Jason Meckley
Programmer
Specialty Bakers, Inc.

faq855-7190
faq732-7259

 

[dinger2121]

that all makes sense. thanks again for the help.

carl
MCSD, MCTS:MOSS

 

[jmeckley]

BTW: kudos for trying to learn a new approach to design/development. learning TDD is akin to speaking another language or writing with the opposite hand. it feels very unnatural at first, but if you stick with it you will begin to see the benefits.

TDD is also something that should be learned from the masters. If you can find a training course on TDD or some books to TDD this will help tremendously. trying to pick it up from scratch, without guidance is extremely difficult.

If you haven't already check out

http://www.lostechies.com

they have alot of bloggers/resources on TDD and other OOP design principles.

Jason Meckley
Programmer
Specialty Bakers, Inc.

faq855-7190
faq732-7259

 

[craigber]

The Art of Unit Testing from Manning is a fantastic book.

Craig Berntson
MCSD, Visual FoxPro MVP,

http://www.craigberntson.com/blog