How to design a method that contains lots of IF's or Switch statements

How can i make the design of this class more dynamic so i can add new extensions and types as required.

public class Processor
{
    public Processor(string fileName)
    {
        string extension = Path.GetExtension(fileName);
        if(extension == "jpg" || extension == "gif" || extension == "png")
        {
            //Process Image file type
        } 
        else if(extension == "xls" || extension == "xlsx")
        {
            开发者_如何学C//Process spreadsheet type
        }
        else if(extension == "doc" || extension == "docx")
        {
            //Process document file type
        }       
        //and so forth ...
    }   
}

We may need to process .tiff files in future or we may need to process video files, which means a new if branch

else if(extension == "avi" || extension == "mp4")
{
    //Process video file type
}

As you can see this can get very long.

The valid files types and groups are stored in a DB...

Can anyone recommend any patterns or clever ideas to solve this problem? Cheers

Use a dictionary.

Dictionary<string, IFileHandler> fileHandlers = new Dictionary<string, IFileHandler>
{
    { "jpg", imageHander },
    { "gif", imageHander },
    { "xls", spreadsheetHander },
    // ...
};

Then use it as follows:

public void Process(string fileName)
{
    string extension = Path.GetExtension(fileName);

    // TODO: What should happen if the filetype is unknown?
    fileHandlers[extension].Process(fileName); 
}

The valid files types and groups are stored in a DB...

Then you probably want to query the database to get the correct group from the extension and use the group as the dictionary key rather than the extension.

I would recommend coming up with an interface for your various file processors to implement:

public interface IFileProcessor
{
    void Process(string fileName);
    IEnumerable<string> FileExtensions {get;}
}

Then use a factory to get the appropriate processor for a particular file name. Like the other answerers, I'm recommending you use a Dictionary to keep a mapping of the processors and their file names. This example uses reflection to register every file processor that it finds in the loaded assemblies. This fits your extensibility requirement well, because all you need is for some loaded assembly to have a class that implements this interface, and it will automatically be registered with the factory. However, you can come up with whatever system you want for registering these types:

public class ProcessorFactory
{
    static IDictionary<string, IFileProcessor> ProcessorsByExtension = 
        new Dictionary<string, IFileProcessor>();

    static ProcessorFactory() 
    {
        var processorTypes = 
            from a in AppDomain.CurrentDomain.GetAssemblies()
            from t in a.GetTypes()
            where typeof(IFileProcessor).IsAssignableFrom(t)
            select t;
        foreach(var t in processorTypes)
        {
            // Preferably use your DI framework to generate this.
            var processor = (IFileProcessor)Activator.CreateInstance(t);
            foreach(var ext in processor.FileExtensions)
            {
                if(ProcessorsByExtension.ContainsKey(ext))
                {
                    throw new InvalidOperationException(
                        "Multiple processors are registered to extension " + ext);
                }
                ProcessorsByExtension[ext] = processor;
            }
        }
    }

    public IFileProcessor GetProcessorForFile(string fileName)
    {
        string extension = Path.GetExtension(fileName);
        return ProcessorsByExtension[extension];
    }   
}

A typical implementation of the file processor interface might look like this:

public class ImageFileProcessor : IFileProcessor
{
    public IEnumerable<string> FileExtensions 
    {
        get {return new[]{"jpg", "gif", "png"};}
    }
    public void Process(string fileName) 
    {
        // Process Image file type
    }
}

This approach keeps your code modular and extensible, while providing excellent performance even if you have an enormous list of file processors.

This structure is generally a bad thing if it's expected to be extended, for the reasons you state (primarily having to to back in and add code branches).

I would implement this as a Dictionary, keyed to the file extension strings, and containing method delegates as values. That would, if properly designed, allow you to add new extensions without affecting old code, since the new code would be in a completely different method that could even live outside the class that has this logic. Then, simply look up the proper delegate to call by the file extension.

Basic example:

//Put this in your class somewhere
public readonly Dictionary<string, Action<FileInfo>> fileHandlers = new Dictionary<string, Action<FileInfo>>();

...

//depending on the dictionary's visibility, you can add these from pretty much anywhere
fileHandlers.Add("xls", ProcessExcelFile);
fileHandlers.Add("xlsx", ProcessExcelFile);
fileHandlers.Add("jpg", ProcessImageFile);

...

//Then all you have to do to invoke the logic is...
fileHandlers[extension](fileInfo);

One way might be to build a table (list), where each item contains the extension along with information about what to do with names with that extension.

You can then write a simple loop, that compares against each extension in the table.

If what you do with each extension is complex and can't be defined in a simple table, then you could instead include a delegate. When the loop finds the matching extension, it could call the corresponding delegate to process the name.

There is a rule for this refactoring--

• Replace condition with Polymorphism/strategy. The obvious problem with if/switch is very likely to have errors in such code, and it is difficult to maintain or enhance.

Considering the same and the "Open-Closed Principal" ( -- Classes should be open for extension but closed for modifications.).

I would suggest for the code below.

public class Processor
    {
        private Dictionary<string,FileParserBase> _fileExtension2FileParser;

        public Processor() {
            _fileExtension2FileParser = new Dictionary<string, FileParserBase>();

            AddParser(new DocExtensionWordParser());
            AddParser(new DocXExtensionWordParser());
            //..more,more
        }

        private void AddParser(FileParserBase fileParserBase) {

            _fileExtension2FileParser.Add(fileParserBase.Extension, fileParserBase);
        }


    public void Process(string fileName)
    {
        string extension = Path.GetExtension(fileName);
        FileParserBase fileParser;
        if (_fileExtension2FileParser.TryGetValue(extension, out fileParser)) {
            fileParser.Process(fileName);
        }

    }
    }

    public interface FileParserBase
    {
        string Extension { get; }
        void Process(string filePath);
    }

    public abstract class WordParserBase : FileParserBase
    {
        private string _extension;

        public WordParserBase(string extension)
        {
            _extension = extension;
        }

        public override void Process(string filePath)
        {
            //Do the processing for WORD Document
        }

        public override string Extension
        {
            get { return _extension; }
        }
    }

    public class DocExtensionWordParser : WordParserBase
    {

        public DocExtensionWordParser():base("doc"){}
    }

    public class DocXExtensionWordParser : WordParserBase
    {

        public DocXExtensionWordParser() : base("docx") { }
    }

for this particular case I'd build something like

List<string> excelExtensions = new List<string>(){ "xls", "xlsx" };
List<string> wordExtensions = new List<string>(){ "doc", "docx" };

if(excelExtensions.Contains(extension))
{
}

if(wordExtensions.Contains(extension))
{
}

etc?