0

I have been working on a program that reads data from various files and stores it into a common database table. The format of data in the files varies based on the data source, making each file unique. I need a way to determine the file reader based on the name of the data source, and I am having trouble avoiding a switch-case statement, even with an abstract factory in place. I know I could inject the IFileReader but that only pushes the switch-case up a level.

Below is an example of my current implementation. How can I completely eliminate the switch-case statement and still dynamically determine the file reader?

public class FileProcessor
{ 
    private readonly IFileReaderFactory _fileReaderFactory;
    private readonly FooDbContext _fooDb;

    public FileProcessor(IFileReaderFactory fileReaderFactory, FooDbContext fooDb)
    {
        _fileReaderFactory = fileReaderFactory;
        _fooDb = fooDb;
    }

    public void ProcessFile(string source, string filePath)
    {
        IFileReader fileReader;
        switch (source)
        {
            case "A":
                fileReader = _fileReaderFactory.CreateReader<FileReaderA>();
                break;
            case "B":
                fileReader = _fileReaderFactory.CreateReader<FileReaderB>();
                break;
            default:
                throw new Exception($"Unknown source: {source}");
        }

        _fooDb.Foos.AddRange(fileReader.ReadFile(filePath));
        _fooDb.SaveChanges();
    }
}

public interface IFileReaderFactory
{
    IFileReader CreateReader<T>() where T : IFileReader, new();
}

public class FileReaderFactory : IFileReaderFactory
{
    public IFileReader CreateReader<T>() where T : IFileReader, new()
    {
        return new T();
    }
}

public interface IFileReader
{
    List<Foo> ReadFile(string filePath); 
}

public class FileReaderA : IFileReader
{
    public List<Foo> ReadFile(string filePath)
    {
        // read file a
        return new List<Foo>();
    }
}

public class FileReaderB : IFileReader
{
    public List<Foo> ReadFile(string filePath)
    {
        // read file b
        return new List<Foo>();
    }
}
shuniar
  • 2,592
  • 6
  • 31
  • 38
  • This can be achieved using reflection. http://stackoverflow.com/questions/232535/how-do-i-use-reflection-to-call-a-generic-method – jrobichaud Sep 08 '16 at 00:33
  • You may want to look into State Pattern. http://www.codeproject.com/Articles/489136/UnderstandingplusandplusImplementingplusStateplusP. Personally, I usually wind up going back to the case statement. –  Sep 08 '16 at 00:40
  • There are way too many options here. You need to do some research and narrow the question. Stack Overflow is already full of questions and answers that address the broad task of selecting from some set of actions based on some input parameter (e.g. a dictionary of delegates) or calling a generic method with a dynamically parameterized type, just two of the many options available to you in this case. – Peter Duniho Sep 08 '16 at 00:42
  • @PeterDuniho I have done plenty of research on this, and many SO questions fall back on the factory, which doesn't work in this scenario. All of the options you mentioned serve essentially the same purpose as the switch-case, they're just different implementations. You're hardcoding a dictionary, or hard coding the parameterized type (e.g. abstract factory), you still need to implement this type of logic somewhere. I'm looking for something that doesn't depend on a hard-coded lookup. Maybe I should rephrase the question if that helps? – shuniar Sep 08 '16 at 00:51
  • @jrobichaud I am aware that I can use reflection, but this requires that I now know the fully qualified name of the class I want to create, which I am hoping to avoid. – shuniar Sep 08 '16 at 00:53
  • If the caller is already providing what the source is, is it possible to just remove the provided source string, make the ProcessFile method generic, and pass in the type of FileReader through that? – Jonathon Chase Sep 08 '16 at 00:53
  • 1
    Since your question currently shows no evidence of any research, nor does it clearly explain why the many existing and related answers do not address your particular scenario, yes...I would say you should definitely rephrase (or rather, completely rewrite) the question. Make sure in addition to explaining what options you've investigated and rejected, you explain why you've rejected them, and provide a good [mcve] that clearly and completely illustrates your specific scenario. – Peter Duniho Sep 08 '16 at 00:54
  • 1
    Here is some search https://www.bing.com/search?q=c%23+refactor+switch to help you with [edit] of the post to narrow it down. – Alexei Levenkov Sep 08 '16 at 00:56
  • @JonathonChase how would the caller determine the `FileReader`? Some sort of switch logic is still required. – shuniar Sep 08 '16 at 00:56
  • If the caller won't know what it's source is ahead of time, then you're going to have to account for the lookup for the proper file reader. I would push the switch logic into the factory as the most appropriate place for it to live. – Jonathon Chase Sep 08 '16 at 00:58
  • @PeterDuniho the question meets the minimal, complete, and verifiable example, it is simply missing my reasons for rejecting other approaches. The code works, and shows the exact problem at hand. My intent was for brevity. – shuniar Sep 08 '16 at 01:00
  • 1
    Also `switch` is not the only way to implement logic of any kind... Since you've mentioned DI you probably should not even write any code but just used named registration of container you like - i.e. http://stackoverflow.com/questions/7046779/with-unity-how-do-i-inject-a-named-dependency-into-a-constructor – Alexei Levenkov Sep 08 '16 at 01:00
  • @shuniar the question is not "missing MCVE", but way too broad and MCVE provided in the post does not in any way helps to narrow down set of possible answers. (Note that lack of research is reason or downvotes which is surprisingly low at this point) – Alexei Levenkov Sep 08 '16 at 01:03

1 Answers1

0

You can eliminate the switch statement from your example by creating an abstract FileProcessorBase class and then creating classes that inherit from it, one for each type of fileReader (see below).

This will get rid of the switch statement from this class, but you'll still need a switch statement to determine which instance of FileProcessorBase you'll require. I don't see that it's possible to entirely eliminate switch statements or conditional logic but if you keep following this process eventually it will get pushed back to the composition root where you compose your application.

Some DI containers support convention based registration, you might be able to use this to completely eliminate your switch statements although there would still be conditional logic.

public abstract class FileProcessorBase
{
    protected  IFileReaderFactory _fileReaderFactory;
    protected  FooDbContext _fooDb;

    public void ProcessFile(string source, string filePath)
    {
        _fooDb.Foos.AddRange(GetFileReader().ReadFile(filePath));
        _fooDb.SaveChanges();
    }

    protected abstract IFileReader GetFileReader();
}

public class FileProcessorA : FileProcessorBase
{
    public FileProcessorA(IFileReaderFactory fileReaderFactory, FooDbContext fooDb)
    {
        _fileReaderFactory = fileReaderFactory;
        _fooDb = fooDb;
    }

    protected override IFileReader GetFileReader()
    {
        return _fileReaderFactory.CreateReader<FileReaderA>();
    }
}

public class FileProcessorB : FileProcessorBase
{
    public FileProcessorB(IFileReaderFactory fileReaderFactory, FooDbContext fooDb)
    {
        _fileReaderFactory = fileReaderFactory;
        _fooDb = fooDb;
    }

    protected override IFileReader GetFileReader()
    {
        return _fileReaderFactory.CreateReader<FileReaderB>();
    }
}
mark_h
  • 5,233
  • 4
  • 36
  • 52