Can this code where it checks on type and then casting be improved?

I have this code but I feel it can be improved. What are your thoughts?

public void MyMethod(object Value)
{
    if (Value.GetType() == typeof(List<Document>))
    {
        var documentList = Value as List<Document>;
        if (MainForm != null)
            MainForm.BindData(documentList);
    }
    else if (Value.GetType() == typeof(Documen开发者_运维技巧t))
    {
        var document = Value as Document;
        if (MainForm != null)
            MainForm.BindData(document);
    }
}

---

It is unlikely that a BindData() method takes anything else but an argument of type object. Or that anything good happens when this method is called with an object that cannot act as a binding source, you'll want to know about it. Thus:

public void MyMethod(object Value)
{
    MainForm.BindData(Value);
}

A form that accepts a binding on both an object and a collection of objects is very unusual. It requires a very different kind of user interface.

---

You could overload the method:

public void MyMethod(List<Document> documentList)
{
   if (MainForm != null)
     MainForm.BindData(documentList);
}

public void MyMethod(Document document)
{       
   if (MainForm != null)
     MainForm.BindData(document);
}

However this is repeating code which isn't advisable either.

---

Instead maybe try parameterising the method:

public void MyMethod<T>(T document)
{       
   if (MainForm != null)
     MainForm.BindData(document);
}

The runtime should dispatch to the correct overload of BindData() without a cast, assuming BindData() is made generic as well:

in MainForm:

public void BindData<T>(T Data) {
  if (T is typeof(Document)) {
    // Bind a document
  } else {
     ...
  }
}

---

Oli's answer is the best one, Mark's is good too. If you just want a single method though here yet another way:

public void MyMethod(object Value)
{
    List<Document> documents = Value as List<Document>;
    if (Value is Document)
    {
        documents = new List<Document>();
        documents.Add((Document) Value);
    }

    if (MainForm != null && documents != null)
        MainForm.BindData(documents);
}

or for a small performance optimization at the expense of succinctness:

public void MyMethod(object Value)
{
    List<Document> documents = null;
    if (Value is List<Document>)
    {
        documents = (List<Document>) Value;
    }
    else if (Value is Document)
    {
        documents = new List<Document>();
        documents.Add((Document) Value);
    }

    if (MainForm != null && documents != null)
        MainForm.BindData(documents);
}

---

first thing you can do is:

public void MyMethod(object Value)
{
    var documentList = Value as List<Document>;
    if (documentList != null)
        MainForm.BindData(documentList);
    else
    {
         var document = Value as Document;
         if (MainForm != null)
            MainForm.BindData(document);
    }
}

But still, Usually a better design can be made for those cases

---

Yes. Don't get into a situation where a variable (Value) could hold an object of two completely unrelated types (List<Document> or Document).

Without seeing how this code is used (i.e. the context), I can't be any more specific, though.

---

I think this is better?

public void MyMethod(object Value)
    {

                            var documentList = Value as List<Document>;
                            if (documentList != null)
                            {
                               if (MainForm != null)
                                MainForm.BindData(documentList);
                            }

                            var document = Value as Document;
                            if (document != null)
                             { 
                            if (MainForm != null)
                                MainForm.BindData(document);
                             }
}

---

In addition to the other answers (whichever you choose) it looks like a good candidate to be made into an extension method, like so:

static class MainFormExtensions
{
    public static void BindData(this MainForm form, object value)
    {
        //Whichever implementation you prefer, E.G.
        MainForm.BindData(value as Document);
    }
}

Then you would be able to call it like this, which is both easy to read and communicates the behaviour of the method.

object value = new Document();
MainForm.DataBind(value);

The best part is that you give the compiler a fair chance to spot know what type your value is. If a year from now you end up calling the method in a strongly typed manor, then the compiler will know to ignore your method and call MainForm(Document document) directly for a sneaky performance boost. Then hopefully one day your (ugly) method will be come redundant and can be deleted.

MainForm.DataBind(new Document());