Removing ifs based on type and list of parameters

Question

I want to refactor following recursive method:

public static void Initialize(Control control, DocumentContainer container, ErrorProvider provider)
{
    if (control == null)
    {
        return;
    }

    var controlWithTextBase = control as ICustomControlWithText;
    if (controlWithTextBase != null)
    {
       controlWithTextBase.DocumentLoaded = true;
       controlWithTextBase.Initialize(container, provider);
    }

    var custom = control as CustomCheckbox;
    if (custom != null)
    {
        custom.DocumentLoaded = true;
        custom.Initialize(container);
    }

    foreach (Control subControl in control.Controls)
    {
        Initialize(subControl, container, provider);
    }
}


public interface ICustomControlWithText : ICustomControl
{
    void Initialize(DocumentContainer container, ErrorProvider provider);
    void InitializeValidations();

    string Text { get; set; }
    ErrorProvider ErrorProvider { get; set; }
    List<IValidation> Validations { get; set; }
}


public interface ICustomControl
{
    void Clear();

    FieldType FieldType { get; set; }
    bool DocumentLoaded { get; set; }
}

class CustomCheckbox : CheckBox, ICustomControl
{
     public void Initialize(DocumentContainer container)
    {
    //...
    }
}

As you can see, depends on type of winforms control this code initialize a control. It starts with main form, and this contains custom controls(IControlWithText,CustomCheckbox) and default winforms forms. I would create 3 Initializators and to every a method CanInitialize depending on type of a control, but even then I have no idea how can I skip those "ifs", which I need to know if I need send this ErrorProvider to method Initialize.

I would be very grateful for your help!

Answer 1

You can use "Dynamic Overload Resolution". (Requires .Net 4+)

If you cast your input control to dynamic, .Net will look for a suitable overload at runtime.
Be careful to provide a "catch" overload for the case of an unexpected type of control. This is what the object overload here is good for. Otherwise you might encounter runtime exceptions.

public static void Initialize(Control control, DocumentContainer container, ErrorProvider provider)
{
    if (control == null) return;

    dynamic c = control;       
    InitializeControl(c, container, provider);

    foreach (Control subControl in control.Controls)
        Initialize(subControl, container, provider);
}


public static void InitializeControl(ICustomControlWithText controlWithTextBase, DocumentContainer container, ErrorProvider provider)
{
    controlWithTextBase.DocumentLoaded = true;
    controlWithTextBase.Initialize(container, provider);
}

public static void InitializeControl(CustomCheckbox custom, DocumentContainer container, ErrorProvider provider)
{
    custom.DocumentLoaded = true;
    custom.Initialize(container);
}

public static void InitializeControl(object _, DocumentContainer container, ErrorProvider provider)
{
    // do nothing if the control is neither a ICustomControlWithText nor a CustomCheckbox
}

Answer 2

I use this pattern all the time:

First, define an initializer interface. This interface will be used to define how types of controls are initialized.

    public interface IInitializer
    {
        void Intialize(Control c, DocumentContainer container, ErrorProvider provider);
        bool Accept(Control c);
    }

Second, create some initializers, each of which represents one of the if statements.

    public class InitializerForControlWithTextBase : IInitializer
    {
        public void Intialize(Control control, DocumentContainer container, ErrorProvider provider)
        {
            var c = control as ICustomControlWithText;
            c.DocumentLoaded = true;
            c.Initialize(container, provider);
        }

        public bool Accept(Control c)
        {
            return GetBaseType().IsInstanceOfType(c);
        }

        public Type GetBaseType()
        {
            return typeof(ICustomControlWithText);
        }
    }

    public class InitializerForCustomCheckbox : IInitializer
    {
        public void Intialize(Control control, DocumentContainer container, ErrorProvider provider)
        {
            var c = control as CustomCheckbox;
            c.DocumentLoaded = true;
            c.Initialize(container);
        }

        public bool Accept(Control c)
        {
            return GetBaseType().IsInstanceOfType(c);
        }

        public Type GetBaseType()
        {
            return typeof(CustomCheckbox);
        }
    }

Third, rewrite the Initialize.

    public static void Initialize(IEnumerable<IInitializer> initializers, Control control, DocumentContainer container, ErrorProvider provider)
    {
        if (control == null) return;

        var inSituInitializer = control as IInitializer;
        if (inSituInitializer != null)
        {
            inSituInitializer.Intialize(control, container, provider);
        }
        else
        {
            foreach (var initializer in initializers)
            {
                if (initializer.Accept(control))
                {
                    initializer.Intialize(control, container, provider);
                    break;
                }
            }
        }

        foreach (Control subControl in control.Controls)
        {
            Initialize(initializers, subControl, container, provider);
        }
    }

You will note that I first checked to see if the control already implemented IIntializer. If it does, then you can do the initialization directly without further seaching (easy to place this on your own code). If using third party controls, you find and use one of the initializers from step 2 above.

Finally, call the initializer somewhere.

            var inits = new IInitializer[] 
            { 
                new InitializerForControlWithTextBase(), 
                new InitializerForCustomCheckbox(), 
            };

            Initialize(inits, control, container, provider);

The benefit of this approach is:

1. You have two ways to find an intializer - put it on your control, or loop through and find one for a third party control. (You can of course improve performance on the loop further with a dictionary if you have hundreds of initializers).

2. The code is decoupled - you can put the individual initializers in a separate DLL.

Answer 3

I would do the following:

1. Move the method void Initialize(DocumentContainer container, ErrorProvider provider); from ICustomControlWithText to ICustomControl

2. Change Initialize method signature in CustomCheckbox from

public void Initialize(DocumentContainer container)

to

public void Initialize(DocumentContainer container, ErrorProvider provider);

Yes, the provider variable will never be used by CustomCheckBox, but so what? It is much cheaper in terms of execution times and code size to add one extra parameter to method call, than use visitors or dynamic methods (IMHO)

3. Rewrite the recursive method the following way:

   public static void Initialize(Control control, DocumentContainer container, ErrorProvider provider)
   {
       if (control == null)
       {
           return;
       }
   
       var custom = control as ICustomControl;
       if (custom != null)
       {
           custom.DocumentLoaded = true;
           custom.Initialize(container, provider);
       }
   
       foreach (Control subControl in control.Controls)
       {
           Initialize(subControl, container, provider);
       }
   }
   

Answer 4

Maybe something based on polymorphism. It's plain and pretty simple I think:

class TempTest
    {
        public static void Run()
        {
            IData data = new InitData() { IntegerData = 1, StringData = "some" };

            IBaseControl c1 = new ControlA();
            IBaseControl c2 = new ControlB();

            c1.Init( data );
            c2.Init( data );
        }
    }


    // Interfaces

    public interface IData
    {
        int IntegerData { get; set; }
        string StringData { get; set; }
    }

    public interface IBaseControl
    {
        void Init( IData data );
    }

    public interface IControlA
    {
        void Init( int IntegerData );
    }

    public interface IControlB
    {
        void Init( int IntegerData, string StringData );
    }


    // Base classes

    public abstract class Base : IBaseControl
    {
        #region IBaseControl Members

        public abstract void Init( IData data );

        #endregion
    }


    // Concrete classes

    public class InitData : IData
    {
        public int IntegerData { get; set; }
        public string StringData { get; set; }
    }

    public class ControlA : Base, IControlA
    {
        public override void Init( IData data )
        {
            Init( data.IntegerData );
        }

        #region IControlA Members

        public void Init( int IntegerData )
        {
            Console.WriteLine( "ControlA initialized with IntegerData={0}", IntegerData );
        }

        #endregion
    }

    public class ControlB : Base, IControlB
    {
        public override void Init( IData data )
        {
            Init( data.IntegerData, data.StringData );
        }

        #region IControlB Members

        public void Init( int IntegerData, string StringData )
        {
            Console.WriteLine( "ControlB initialized with IntegerData={0} and StringData={1}", IntegerData, StringData );
        }

        #endregion
    }

I think you get the idea.

Answer 5

I would agree with @3dGrabber, but here is one more solution without need of dynamic, but very similar to 3dGrabber's answer. But strictly for older version of .net.

public class ClassInitiator{

public static void Initialize(Control control, 
     DocumentContainer container, ErrorProvider provider)
{
    if (control == null) return;

    typeof(ClassInitiator).InvokeMember(
        "InitializeControl",
        BindingFlags.InvokeMethod | BindingFlags.Public,
        null,
        null,
        new object[]{
             control,
             container,
             provider
        });

    foreach (Control subControl in control.Controls)
        Initialize(subControl, container, provider);
}

public static void InitializeControl(
    ICustomControlWithText controlWithTextBase, 
    DocumentContainer container, 
    ErrorProvider provider)
{
    controlWithTextBase.DocumentLoaded = true;
    controlWithTextBase.Initialize(container, provider);
}

public static void InitializeControl(
    CustomCheckbox custom, 
    DocumentContainer container, 
    ErrorProvider provider)
{
    custom.DocumentLoaded = true;
    custom.Initialize(container);
}

public static void InitializeControl(
    object _, 
    DocumentContainer container, 
    ErrorProvider provider)
{
    // do nothing if the control is neither a 
    // ICustomControlWithText nor a CustomCheckbox
}
}

Note, this is only to support previous version of .NET 2.0 which did not have dynamic support, for performance, dynamic is faster because it caches runtime resolution needed to invoke method, but in this case InvokeMember's resolution takes longer.

Alternative would be to resolve Method and cache it as shown below.

private Dictionary<Type,MethodInfo> cache = 
   new Dictionary<Type,MethodInfo>();


public static void Initialize(Control control, 
     DocumentContainer container, ErrorProvider provider)
{
    if (control == null) return;

    MethodInfo initializer = null;
    Type controlType = control.GetType();
    if(!cache.TryGetValue(controlType, out initializer)){
        initializer = typeof(ClassInitialor).GetMethod("InitializeControl",
            new Type[] {
                controlType,
                typeof(DocumentContainer),
                typeof(ErrorProvider),
            });
        cache[controlType] = initializer;
    }

    initializer.Invoke(null,
        new object[] {
             control,
             container,
             provider
        });

    foreach (Control subControl in control.Controls)
        Initialize(subControl, container, provider);
}

Answer 6

What you are looking for is the Visitor (Gang of Four) pattern.

Make sure your base interface ICustomControl accepts visitors by adding an extra Accept method to it. Let that visitor be named ControlVisitor but any other name would do.

    public interface ICustomControl
    {
        void Accept(ControlVisitor visitor);
        void Clear();

        FieldType FieldType { get; set; }
        bool DocumentLoaded { get; set; }
    }

Simplify the Initialize method

    public static void Initialize(Control control, ControlVisitor visitor)
    {
        if (control == null) //can this ever be null?
        {
            return;
        }

        var customControl = control as ICustomControl;
        if (customControl != null)
        {
           customControl.Accept(visitor);
        }


        foreach (Control subControl in control.Controls)
        {
            Initialize(subControl, visitor);
        }
    }

And fill in the blanks by adding a visitor (my ControlVisitor example is a concrete one but you can have an interface for it as well). Here you will provide an overloaded Visit method

    public class ControlVisitor
    {
        private readonly DocumentContainer container;
        private readonly ErrorProvider provider;
        public ControlVisitor(DocumentContainer container, ErrorProvider provider)
        {
            this.container = container;
            this.provider = provider;
        }

        public void Visit(ICustomControlWithText control)
        {
            control.DocumentLoaded = true;
            control.Initialize(container, provider);
        }

        public void Visit(CustomCheckbox control)
        {
            control.DocumentLoaded = true;
            control.Initialize(container);
        }            
    }

The implementation of the Accept method is quite simple and the same wherever you do it. Below the example for CustomCheckBox :

    public class CustomCheckbox : CheckBox, ICustomControl
    {
        //..

        public void Accept(ControlVisitor visitor)
        {
            visitor.Visit(this);
        }
        //..
    }

Answer 7

Remove the Initialization logic from the second interface and leave only the validation logic,so you can write something like this:

public interface ICustomControl
{
        void Clear();
        FieldType FieldType { get; set; }
        bool DocumentLoaded { get; set; }
        void Initialize(DocumentContainer container);
}

public interface ICustomControlWithText 
{
        string Text { get; set; }
        ErrorProvider ErrorProvider { get; set; }
        List<IValidation> Validations { get; set; }
        void Validate(ErrorProvider provider);
}



class CustomCheckbox : CheckBox, ICustomControl    {  ...  }

class CustomTextbox : TextBox, ICustomControl, ICustomControlWithText      {...}

 public static void Initialize(Control control, DocumentContainer container, ErrorProvider provider)
    {
        var custom = control as ICustomControl;

        if (control == null)
            return;

        custom.DocumentLoaded = true;
        custom.Initialize(container);

        var controlWithTextBase = control as ICustomControlWithText;
        if (controlWithTextBase != null)
            controlWithTextBase.Validate(provider);

        foreach (Control subControl in control.Controls)
            Initialize(subControl, container, provider);
    }

Of course you can make ICustomControlWithText "inherit" ICustomControl if you want

public interface ICustomControlWithText : ICustomControl