Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

Say I have a class Person:

class Person
{
  string FullName;
  int Age;
}

And I need to load a list of Person's from a plain text file. The text file could be in one of two formats. I want a method such as LoadFromFile(string path) that will 1) detect the file format 2) parse appropriately for the file type and 3) return a list of Persons.

The quick and dirty solution would be to have a single method LoadFromFile where I read the header of the file, I can determine which file type it is, and then, keeping the file open, switch on the type and read it appropriately.

On the other hand, the fact that it's two different "types" of files makes me think it's a good candidate for polymorphism. This would be especially good if there was a third file type. However, having separate classes makes it seem like it would be harder to keep the file open from the "type detection" phase to the "parse phase". Though maybe wanting to keep the file open is just premature optimization.

I want the client code to simply call LoadFromFile(path) and have it detect the file type and load appropriately.

What is a good design for this (in terms of class names and method names)?

share|improve this question

3 Answers 3

up vote 3 down vote accepted

Strategy Pattern

The strategy pattern defines a family of algorithms, encapsulates each one, and makes them interchangeable. Strategy lets the algorithm vary independently from clients that use it.

Here's some excellent examples of the strategy pattern

As you can see from the 1st link above, you can detect the file type in the header in your main method, and then feed the correct version of the Strategy into the context and then call the context.

If you really want to you could write a Factory Method to automatically create your context.

So you could call something like the following

var context = FileFactory.CreateContext(myFile);
context.Parse();

A factory method might be a little overkill for this however, this would be a judgement call on your part. However using the strategy pattern as above gives you the flexibility to add any number of new file types to parse. Additionally the factory will give you the ability to abstract away all of the polymorphism from your logic code.

Edit

Here is what you would end up with using a strategy pattern combined with a Factory Method.

public interface IFileParserStrategy
{
    IList<Person>  Parse(String path);
}


public class FileFormatNo1 : IFileParserStrategy
{    
   private string _path; 
    public FileFormatNo1(String path){
      _path = path;
    }

    public IList<Person> Parse()
    {
        // Actual Logic for handling File Format #1 
        // and returning a IList<Person>   
    }
}

public class FileFormatNo2 : IFileParserStrategy
{     
    private string _path; 
    public FileFormatNo1(String path){
      _path = path;
    }

    public IList<Person> Parse(String path)
    {
        // Actual Logic for handling File Format 2
        // and returning a IList<Person>   
    }
}

public interface IFileParserContext 
{ 
    public IList<Person> Parse();
}

public class FileParserContext : IFileParserContext
{
    private IFileParserStrategy _strategy;

     public FileParserContext(IFileParserStrategy fileParserStrategy)
     {
         _strategy = fileParserStrategy;
     }

     public IList<Person> Parse()
     {
         return _strategy.Parse();
     }
}

public static class FileParserFactory
{
    public static IFileParserContext CreateContext(String path)
    {
           FileParserContext context = null;

           var checkFileFormat = File.ReadAllLines();

            if(checkFileFormat.Contains("FileFormatNo1"))
              context = new FileParserContext(new FileFormatNo1(path));

            if(checkFileFormat.Contains("FileFormatNo2"))
               context = new FileParserContext(new FileFormatNo2(path));

             if(context == null)
                throw new FileFormatException("File is not in a readable state");

             return context;            
    }       
}


public static void Main()
{    
    var filePath = "someFile.txt";
    var context = FileParserFactory.CreateContext(filePath);    
    var personList = context.Parse();
}

Update

You could modify the above strategy to pass around a FileStream if you only wanted to open the file once. You could modify FileParserFactory.CreateContext to accept a FileStream for its parameter, and refactor the strategies to accept the FileStream instead of the filePath.

public static void Main()
{    
    var filePath = "someFile.txt";
    List<Person> personList;

    using(var fileStream = new FileStream(filePath, FileMode.Open, FileAccess.Read, FileShare.Read)) 
    {
        var context = FileParserFactory.CreateContext(fileStream);    
        personList = context.Parse();
    }        
}
share|improve this answer
1  
I would recommend this approach. It leverages polymorphism and decouples the clients from the implementation. OO design the way it was meant to be. –  Thomas Owens Jul 23 '11 at 10:18
    
I like it. But you side step the "keeping the file open" across the file type detection to file parsing phases. Do you think that it's just cleaner to do that? I don't really care about opening the file twice but if there's an elegant solution.. –  User Jul 26 '11 at 19:37
    
You could modify the above strategy to have FileParserFactory.CreateContext to pass around a FileStream. Have main open and close your filestream. FileStream s2 = new FileStream(name, FileMode.Open, FileAccess.Read, FileShare.Read); Regardless of which way you choose to either open the file once or twice, using the strategy is the way I would achieve this outcome. I've updated the answer with a short snippet of what I mean, you should be able to extrapolate the rest. –  Justin Shield Jul 26 '11 at 23:10
  1. Open file
  2. Read header
  3. Determine type
  4. Dispatch into one of two specialized loader functions, passing it the already-opened file handle
  5. Close file

The dipatcher can be a simple switch statement, or, if the language allows it and it makes sense, a dictionary mapping header types to function pointers (delegates, anonymous functors, ...).

Step 5, I think should be done in the main loader, not the format-specific ones: you'll have to close the file anyway, and this one you can elegantly put it in a finally block in case the loader throws an exception.

Another consideration is whether or not you should rewind the file before calling the format-specific loader; if you intend to support completely incompatible formats; rewinding means your format-specific loader can be called directly when you already know the format, but it also requires reading the header twice (or skipping it the second time, if it's fixed-size). Also, if you want to read from a source that doesn't allow seeking backward (network socket, pipe, etc.) you can't do this (unless you buffer the input stream).

This way, you'll separate functionality in a way that makes sense, without any fluff. It's also rather maintainable - if you need to add another file format, just write the loader function and add it to the dispatcher. For a bit more encapsulation, you can pull the dispatcher out of the main loader and put it into its own function (in the spirit of the factory pattern).

If you want, you can OOP the whole thing - wrap the main loader into a class, write an interface for the individual loaders, make the loaders classes that implement the loader interface, make the dispatcher a factory class that returns a loader. The basic idea is the same though; you building blocks are main loader, format-specific loaders, and a dispatcher.

share|improve this answer

the polymorphic loader is an interesting setup (and can get you some extra points for an assignment ;) )

this would mean you need an interface with isThisFormat() load(istream) and a implement a class for each format

the main issue that I can see is how to create the detection

one way would be to assume the first x bytes are enough to detect it and simply read and pass that (and reset the stream afterwards)

another way would be to pass the stream itself and ensure the (seekable/markable) stream is reset to the start of the file when returning false (as part of the method contract)

then you can call it like

List<Person> LoadFromFile(string path){
    istream stream = ;//open file
    try{
        foreach(PersonLoader loader in registeredPersonLoaders){
             if(loader.isThisFormat(stream)){//looping is the type detection
                 return loader.load(stream);//the if branch is the parse phase
             }
        }
        throw UnkownFormatException(path);//or return empty_list
    }finally{
        stream.close();//cleanup
    }
}

this way you don't need to worry that the file will remain open and the implementation of PersonLoader doesn't need to worry about that

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.