0

I've posted my factory to codereview.so now I've got this:

public class WpfControlFactory
{
    public static TControl CreateWpfControl<TControl>(string name = null) where TControl : class, IWpfControl
    {
        TControl wpfControl = default(TControl);

        //Avoid some bone-headed exceptions
        if (!typeof(TControl).IsAbstract)
        {
            wpfControl = Activator.CreateInstance<TControl>();
        }

        if (wpfControl != null)
        {
            wpfControl.Name = name ?? Consts.DefaultEaControlName;
        }

        return wpfControl;
    }
}

But unfortunately I can't use CreateWpfControl<TControl>() because I don't have TControl I've got only typeName string.

I've read this so I know how to create generic method with reflection. But actually I don't know where should I create it. In factory like this:

    public static IWpfControl CreateWpfControl(string controlType, string controlName)
    {
        Type type = FindType(controlType);
        if (type == null)
        {
            return null;
        }

        MethodInfo method = typeof(WpfControlFactory).GetMethod("CreateInstance");
        MethodInfo generic = method.MakeGenericMethod(type);
        return (IWpfControl)generic.Invoke(null, null);
    }

    private static Type FindType(string typeName)
    {
        Type type = null;
        WpfControl wpfControl;
        Enum.TryParse(typeName, out wpfControl);
        if (wpfControl != default(WpfControl))
        {
            type = Type.GetType(typeName);
        }

        return type;
    }

    private static TControl CreateInstance<TControl>(string name = null) where TControl : class, IWpfControl
    {
        TControl wpfControl = default(TControl);

        //Avoid some bone-headed exceptions
        if (!typeof(TControl).IsAbstract)
        {
            wpfControl = Activator.CreateInstance<TControl>();
        }

        if (wpfControl != null)
        {
            wpfControl.Name = name ?? Consts.DefaultEaControlName;
        }

        return wpfControl;
    }

Or where? I want my class be consistent with SOLID

EDIT

Next possible version:

public class WpfControlFactory
{
    public static IWpfControl CreateWpfControl(string controlType, string controlName = null)
    {
        IWpfControl wpfControl = default(IWpfControl);

        Type type = Type.GetType(controlType);
        if (type != null && type.IsAssignableFrom(typeof(IWpfControl)))
        {
            wpfControl = (IWpfControl)Activator.CreateInstance(type);
        }

        if (wpfControl != null)
        {
            wpfControl.Name = controlName ?? Consts.DefaultEaControlName;
        }

        return wpfControl;
    }
}
Community
  • 1
  • 1
rechandler
  • 756
  • 8
  • 22
  • 2
    Could you say something about the line with `Enum.TryParse`? I can't get the idea. Plus, I don't quite get why you insist on reusing the generic version. Having the type name in hand, you could use the other overload of `CreateInstance` that accepts the type name as string. – Wiktor Zychla Nov 01 '14 at 10:00
  • @WiktorZychla I have enum of supported controls, this is important because I'm writing addin to Enterprise Architect and I don't support all controls that it provide. So I check if my controlType is supported type. Edit version in first post. – rechandler Nov 01 '14 at 10:24

1 Answers1

0

Your second approach won't work, you can't get the type out of the controlName. My idea is that if you already have a closed enum, make use of it.

public class WpfControlFactory
{
  public static IWpfControl CreateWpfControl(WpfControl control, string controlName)
  {
    IWpfControl wpfControl = default(IWpfControl);

    Type type = GetControl(control);
    if (type != null && type.IsAssignableFrom(typeof(IWpfControl)))
    {
        wpfControl = (IWpfControl)Activator.CreateInstance(type);
    }

    if (wpfControl != null)
    {
        wpfControl.Name = controlName ?? Consts.DefaultEaControlName;
    }

    return wpfControl;
  }

  private Type GetControl( WpfControl control )
  {
     switch ( control )
     {
        case WpfControl.Foo : return typeof( FooControlType );
        ...
     }
  }
}

This way you have a clean 1-1 mapping between enum values and control types.

On the other hand, if you rely on a string, finding a type is much more difficult. Types can exist in different assemblies. A short type name could just be not enough. And making the caller using a full type name (that also includes the assembly name) would make the invocation more difficult (the caller would have to know the type to get its full name).

rechandler
  • 756
  • 8
  • 22
Wiktor Zychla
  • 47,367
  • 6
  • 74
  • 106
  • At the beginning I also try approach with `swich case` but on [codereview.so](http://codereview.stackexchange.com/questions/68559/factory-pattern-with-controls) wasn't very satisfied. I know about create types limitation, and all my controls I want to create, are in the same assembly with factory. So short type is enough I think. – rechandler Nov 01 '14 at 11:05