CODEWITHSUNDEEP
c#class
Tomas
Tomas

Reputation: 18117

c# programming code optimization

I have about 20 classes which are derived from ConvertApi classes. Every class share Convert method from parent class and has unique property SupportedFiles. I use these classes for file manipulation and my code looks like

if (fileEx=="txt")
   new Text2Pdf().Convert(file)
else
  if (fileEx=="doc")
    new Word2Pdf().Convert(file)
     else
    //and so on..

I know that these code can be optimized because 20 times if operator is repeated and that looks bad, but can't find a way to do that. Could anyone help me?

class Text2Pdf : ConvertApi
{
  enum SupportedFiles { txt, log }; 
}

class Word2Pdf : ConvertApi
{
  enum SupportedFiles { doc, docx }; 
}

class Excel2Pdf : ConvertApi
{
  enum SupportedFiles { xls, xlsx }; 
}

class ConvertApi
{
 public void Convert(....);
}

Upvotes: 0

Views: 680

Answers (8)

Louis Kottmann
Louis Kottmann

Reputation: 16628

You can use some reflection:

ConvertApi FindMatchingConverter(string _FileExt)
        {
            //Get all the converters available.
            List<ConvertApi> converters = this.GetType()
                                           .Assembly
                                           .GetExportedTypes()
                                           .Where(type => type.IsAssignableFrom(typeof(ConvertApi)))
                                           .ToList();

            //Loop and find which converter to use
            foreach (var converter in converters)
            {
                if (converter.SupportedFiles.Contains(_FileExt))
                    return Activator.CreateInstance(converter);
            }
            throw new Exception("No converter found");
        }

Then you just have to call Convert() on the ConvertApi returned.
Of course, this requires you to add a virtual List<String> named SupportedFiles in your base class.

This makes it look like

public abstract class ConvertApi
{
    public abstract void Convert();

    public virtual List<String> SupportedFiles {get;set;}
}

Upvotes: 1

David Brabant
David Brabant

Reputation: 43499

You need to use an abstract factory pattern, here. All your text converters should derive from a common interface ITextConverter implementing your Convert method. The file extension would be a parameter of your factory. Here is a sample below (typed "on the fly", sometimes copy-pasting code from my sources, so there might be typos. The goal here is to give you a general idea for a flexible implementation).

public interface IFileConverter
{
    bool Convert(string filePath);
}


public static class FileConverterFactory
{
    public static IFileConverter Create(string extension)
    {
        extension = type.ToUpper();

        Dictionary<string, ConverterConfig> availableConverters = GetConvertersConfig();

        if (!availableConverters.ContainsKey(extension))
            throw new ArgumentException(string.Format("Unknown extenstion type '{0}'. Check application configuration file.", extension));

        ConverterConfig cc = availableConverters[extension];
        Assembly runnerAssembly = Assembly.LoadFrom(cc.Assembly);
        Type converterType = runnerAssembly.GetType(cc.Class);

        IFileConverter converter = (IFileConverter) Activator.CreateInstance(converterType);

        return converter;
    }


    private static Dictionary<string, ConverterConfig> GetConvertersConfig()
    {
        var configs = (Dictionary<string, ConverterConfig>) ConfigurationManager.GetSection("ConvertersConfig");

        return configs;
    }
}


public class ConvertersConfigHandler : IConfigurationSectionHandler
{
    public object Create(object parent, object configContext, XmlNode section)
    {
        Dictionary<string, ConverterConfig> converters = new KeyedList<string, ConverterConfig>();
        XmlNodeList converterList = section.SelectNodes("Converter");

        foreach (XmlNode converterNode in converterList)
        {
            XmlNode currentConverterNode = converterNode;

            ConverterConfig cc = new ConverterConfig();
            cc.Type = XML.GetAttribute(ref currentConverterNode, "Type").ToUpper();
            cc.Assembly = XML.GetAttribute(ref currentConverterNode, "Assembly");
            cc.Class = XML.GetAttribute(ref currentConverterNode, "Class");

            converters[cc.Type] = cc;
        }

        return converters;
    }
}


public class ConverterConfig
{
    public string Type = "";
    public string Assembly = "";
    public string Class = "";
}


public class TextConverter : IFileConverter
{
   bool Convert(string filePath) { ... }
}


public class PdfConverter : IFileConverter
{
   bool Convert(string filePath) { ... }
}

In your app.config file, you add this to the configSections:

<section name = "ConvertersConfig" type = "ConvertersConfigConfigHandler, MyAssembly" />

and this below your configSections:

<ConvertersConfig>
    <Converter Type="txt" Assembly="MyAssembly" Class="MyAssembly.TextConverter" />
    <Converter Type="pdf" Assembly="MyAssembly" Class="MyAssembly.PdfConverter" />
</ConvertersConfig>

The call would then be like this:

IFileConverter converter = FileConverterFactory.Create("txt");
converter.Convert("c:\temp\myfile");

EDITED the code for giving a solution that is more "generic".

Upvotes: 1

Chris Gessler
Chris Gessler

Reputation: 23113

Slower, but more dynamic... use an Object Factory. Here's a good article that seems to fit your needs.

http://www.codeproject.com/Articles/12986/Generic-Object-Factory

The important stuff from the article:

using System.Collections.Generic;

public struct Factory < KeyType, GeneralProduct > 
{
    //Register a object with the factory
    public void> Register< SpecificProduct >(KeyType key)
        where SpecificProduct : GeneralProduct, new()
    {
        if( m_mapProducts == null )
                {
            m_mapProducts = new SortedList< KeyType, CreateFunctor >(); 
        }
        CreateFunctor createFunctor = Creator<SpecificProduct>; 
        m_mapProducts.Add(key, createFunctor);
    }

    //Create a registered object 
    public GeneralProduct Create( KeyType key )
    {
        CreateFunctor createFunctor = m_mapProducts[ key ];
        return createFunctor(); 
    }

    private GeneralProduct Creator < SpecificProduct >() 
        where SpecificProduct : GeneralProduct, new()
    {
        return new SpecificProduct();
    }

    private delegate GeneralProduct CreateFunctor(); 

    private SortedList<KeyType, CreateFunctor>  m_mapProducts;
}

Usage:

class Fruit
{ 
} 

class Apple : Fruit 
{ 
} 

class Orange : Fruit 
{ 
} 

class TestClass
{ 
    static void Main(string[] args) 
    { 
        General.Factory< string, Fruit > factory; 

        //Register
        factory.Register< Apple >( "Apple" ); 
        factory.Register< Orange>( "Orange" ); 

        //Create 
        Fruit fruit1 = factory.Create("Apple"); 
        Fruit fruit2 = factory.Create("Orange"); 
    } 
}

Upvotes: 0

Sergii Kudriavtsev
Sergii Kudriavtsev

Reputation: 10512

  1. C# supports switch-case operator for strings, i.e. your code coud be rewritten as

    switch (fileEx)
{
    case "txt" : new Text2Pdf().Convert(file); break;
    case "doc": new Word2Pdf().Convert(file); break;
    ...
}

  2. If you change your classes' names to correspond to supported extensions then you will be able to construct them using reflection, like this (error checking omitted for brevity):

        var t = Type.GetType(fileEx + "2Pdf");
    var tConstructor = t.GetConstructor(Type.EmptyTypes);
    var tInstance = tConstructor.Invoke(new object[0]);
    ((ConvertApi)tInstance).Convert(...);

This might require some additional work (i.e. create a separate class for each extension, deriving them from some base class - for example Doc2Pdf and Docx2Pdf both deriving from Word2Pdf).

The advantage is that you will not have to touch this part of code anymore. If you're planning to write some interface for plugins then it may come in handy.

The code above also has an assumption that your ConvertApi classes all have default parameterless constuctor.

Upvotes: 1

Moo-Juice
Moo-Juice

Reputation: 38825

In your base class, have something like this:

public abstract class ConvertApi
{
    protected abstract string[] SupportedFilesImpl();

    public bool Supports(string ext)
    {
        return SupportedFilesImpl.Contains(ext);
    }
}

Now, your derived classes can implement this method:

public class Text2PDF : ConvertApi
{
    protected override string[] SupportedFilesImpl { return new string[] { "txt", "log"}; }
}

public class Doc2PDF : ConvertApi
{
    protected override string[] SupportedFilesImpl { return new string[] { "doc", "docx"}; }
}

... and so on for the rest of the converters. Then put these in a list...

List<ConvertApi> converters = new List<ConvertApi>();
converters.Add(new Text2PDF());
converters.Add(new Doc2PDF());

(Note, I'd probably have a class containing these rather than just a list, but anyway). Now, to find a converter:

foreach(ConvertApi api in converters)
{
    if(api.Supports(fileExt))
    {
        // woo!
        break;
    }
}

Upvotes: 3

Jon Skeet
Jon Skeet

Reputation: 1500893

Assuming each converter is stateless, it sounds like you just want a Dictionary<string, ConvertApi>:

private static readonly Dictionary<string, ConvertApi> ConverterByType =
    new Dictionary<string, ConvertApi>
{
    { "txt", new Text2PdfConverter() },
    { "doc", new Word2PdfConverter() },
    ...
};

...

ConvertApi converter;
if (!ConverterByType.TryGetValue(fileEx, out converter))
{
    // No converter available... do whatever
}
else
{
    converter.Convert(file);
}

(The dictionary initialization will end up creating more converters than you really need for any converter that supports multiple extensions, but that's a different matter.)

If you need a new converter each time, make it a Dictionary<string, Func<ConvertApi>> and populate it as:

{ "txt", () => new Text2PdfConverter() },
{ "doc", () => new Word2PdfConverter() },

... then call the delegate to get the converter when you need it.

Of course, this puts all the initialization in one place - you may want a way of getting the converters to register the extensions they understand with a "converter provider" of some type.

Upvotes: 2

Andy Stannard
Andy Stannard

Reputation: 1703

Use dependency injection where you pass in the supported files in the constructor of the class.

Upvotes: 0

JP Hellemons
JP Hellemons

Reputation: 6047

What about using a switch?

switch(fileEx){
    case "txt": new Text2Pdf().Convert(file); break;
    case "doc": new Word2Pdf().Convert(file); break;
}

Upvotes: 0

Related Questions