Possible weaknesses regarding using reflection in a business application

Question

Hi,

Experimenting using different approaches and reducing code maintenance I have ended up using reflection to create new forms in my MDI application.

The reason for this 'solution' is to provide a central place to setup new forms, implement security checks and possible future needs.

I currently have a static Activator class that is instantiated with the MDI parent form, and whenever I need to create setup new forms for the application i call a method i have created : private static T CreateForm<T>(params args[]) where T: class. This method uses the System.Activator class to instantiate the given class. I have other static methods like public static void ShowCustomers() that in reality uses the 'reflection' method by calling CreateForm<frmCustomers>() which sets up the MDI parent and so on.

The object creation occurs within a try/catch block and I show a message if the form couldn't be created and the reasons why.

What I've explained so far does possibly not validate the need to use reflection to create MDI forms in a business application, but I wish to add another use this has in my application. I have a component that implements the ISupportInitialize interface, and when I drop this onto a form it performs security checks and throws a System.Security.SecurityException in the EndInit method. This exception is cought in the CreateFormmethod and a user friendly message is displayed to the user.

Also, I was thinking of possibly storing a MRU (most recently used) list of the forms that are created and where better to do this than in the CreateForm method.

I'm not familiar with terms like code smell, design smell or such but I've seen the words used often enough, but my question basically boils down to this:

Given the information i've provided (hopefully understandable), does this approach smell 'bad'?

If so, which approach is better suited?

Any comments are welcome.

Thanks, Stefan

public class Activator
{
    private static Activator instance;

    private MainForm mainForm;

    private Activator(MainForm mainForm)
    {
        this.mainForm = mainForm;
    }

    private Activator()
    {

    }         

    private static Activator Instance
    {
        get
        {
            if (instance == null) throw new Exception("Not activated");
            else return instance;
        }
    }

    private static void ShowMDIChild<T>(params object[] args) where T : class
    {
        try
        {
            System.Windows.Forms.Form frm = Create<T>(args) as System.Windows.Forms.Form;
            ShowMDIChild(frm);
        }
        catch (Exception e)
        {
            // Check if the inner exception is a security exception
            bool isSecurity = false;
            if (e.GetType().Equals(typeof(System.Security.SecurityException))) isSecurity = true;

            if (!isSecurity && e.InnerException != null && e.InnerException.GetType().Equals(typeof(System.Security.SecurityException))) isSecurity = true;

            if(isSecurity)
                MessageBox.Show(Instance.mainForm, "You do not have the neccessary privileges to access this resource", "Access denied", System.Windows.Forms.MessageBoxButtons.OK, System.Windows.Forms.MessageBoxIcon.Stop);
            else
                MessageBox.Show(Instance.mainForm, e.Message, "An error has occurred", System.Windows.Forms.MessageBoxButtons.OK, System.Windows.Forms.MessageBoxIcon.Error);
        }
    }

    private static void ShowMDIChild(System.Windows.Forms.Form form)
    {
        Instance.mainForm.ShowMDIChild(form);
    }

    private static T Create<T>(params object[] args) where T: class
    {
        T result = System.Activator.CreateInstance(typeof(T), args) as T;

        return result;
    }

    public static void Register(MainForm mainForm)
    {
        instance = new Activator(mainForm);
    }

    public static void Customers()
    {
        ShowMDIChild<Forms.Customers>();
    }
}

Answer 1

If it has no parameters for the constructor you can do something like this. If this is not the case, I see no problems with your design unless you run into performance issues (I doubt it).

public static TForm CreateForm<TForm>() where TForm : Form, new()
{
    return ProccessNewForm<TForm>(new TForm());
} 

public static TForm CreateForm<TForm>(Func<TForm, TForm> initializer) where TForm : Form, new()
{
    return ProccessNewForm<TForm>(initializer(new TForm()));
} 

private static TForm ProccessNewForm<TForm>(TForm newForm) where TForm : Form, new()
{
    // do stuff

    return newForm;
}

Answer 2

You built yourself a framework. It solves your problem and, as you said, it reduces code maintenance. If you are comfortable with code, you're done.

Good idea would be to document it, just in case you get/hire more developers to work on this project.

Answer 3

You could move all of your constructor logic to an interface implementation like IConstruct and add another constraint to CreateForm to allow something like this:

private static T CreateForm<T>(params object[] args) where T : class, new() {
    T t = new T();
    var construct = t as IConstruct;
    if (construct != null) {
        construct.Construct(args);
    }
    var initialize = t as IInitialize;
    if (initialize != null) {
        initialize.Initialize();
    }
    return t;
}

This would achieve the same goals without reflection, but requires moving some constructor logic.