Launch a form based on a parameter

smickell,
from where do you get the 'inFormType' parameter in your method? does that come from a menuitem you have created for your child window?
i think, there is a more efficient way for your needs.
please explain your logic on when your child windows should be created.

vo1d

Thanks sumix, I got it working :) My foreach loop was indeed very wrong.
I modified it a little so that I no longer need to pass in the full string including namespace, because I want as much of my code to be copy-and-pastable into other projects as easily as possible.

public void showForm(string inFormType)
        {   // checks if an instance of a form is already open, and opens it instead of creating a new instance
            // made public so that other child forms can display the lists too

            Form launchForm;
            inFormType = this.GetType().Namespace.ToString() + "." + inFormType;
            Form[] childForms = this.MdiChildren;   // Get all of the MDI children in an array
            bool exists = false;
            foreach (Form chform in childForms)
            {
                if (chform.GetType().FullName == inFormType)  // check if one instance of the form is already opened
                {
                    exists = true;
                    chform.Activate();
                    break;
                }
            }
            if (!exists)
            {
                launchForm = (Form)Activator.CreateInstance(Type.GetType(inFormType));
                launchForm.MdiParent = this;
                launchForm.Show();
            }
        }



vo1d - inFormType is produced from: showForm("formCustomerList"). It is called from each of the main buttons I have on my parent MDI form which calls e.g. customers, suppliers, parts forms etc.
sumix - you will get at least a share of the points for your efforts.

But I am now wondering whether overall this is the most efficient way to go about this? Ideally I would like to be able to pass a form object without creating a new instance of the form first - i.e. showForm(formCustomerList.something)  rather than showForm(new formCustomerList()); Or if you can suggest an even better approach, that would be great too :)

k, then i would recommened the following, except, you are using framework1.1:

create a collection from collectionbase, which will hold your form-instances.(to test it, you could use an arraylist first instead of creating your formscollection).
create that list-object in your mainform.
now, if a button is pressed for one of your childforms, create that form an add the reference to that list.
now, if someone closes a childform, it is removed from the mainwindows MdiChildren array but not from your forms-list.
the advantag of that is, that the garbage collection will not dispose that form cause you still referencing that object in your list ;)
if you click the same button again, you just have to get the formreference from your list and show it again.

if you dont want to itereat through your forms-list each time, a button is clicked, you could save the index, where the form is saved in the list, in the buttons tag object.
so if you click a button, it could look something like this(some sort of pseudocode):

void ButtonClickEvent(sender, eventargs)
{
  Button clickedBtn = sender as Button;
  if(clickedBtn.tag != null)                                                              //we got an index for a form in our list
  {
    Form formIntance = yourFormsList[(int)clickedBtn.Tag] as Form; //the as operator is only needed if you are using an
                                                                                                  //arraylist instead of an own formscollection. in
                                                                                                  //your own formscollection, you could return a Form object
                                                                                                  //in the indexer property, so a cast is not needed.
    if(formInstance != null)                                                            //it should not be null but check because of codeconsistents
    {
               formInstance.MdiParent = this;
               formInstance.Show();
    }
  }
  else                                                                                          //1st time, we have to create our form instance
  {
     Form formInstance = new Form();
     formInstance.MdiParent = this;
     clickedBtn.Tag =  yourFormsList.Add(formInstance);                  //add the formreference to the list and save the index to the tag
     formInstance.Show();
  }
}

this implementation should be faster. one comment to casts: if you have to cast classes, you should use the as operator. it produces less opcode. the other difference of the as operator towards the () cast is that you will get null if the cast fails.

feel free to ask if something is unclear.
regards, vo1d

oh i forgot to say, you have to add this eventhandler to each button clickevent you got on your mainform.
vo1d

vo1d - a couple of things before I delve into this:
I am using .NET 2.0. I am not sure what you meant by your first sentence. Does this help the issue?
Also, I like your idea of keeping the reference in a list - very smart, but would it not actually be better to have the garbage collection dispose of the form? If the user has closed the form it is possible that they might not want it again, so what is the point in still keeping a reference to it? If, say, the user is currently viewing customers and then clicks the 'suppliers' button on the parent form, I don't want the customers form to be closed, it should still be 'under' the supplier form.

I am assuming you are referring to the KB815707 article: http://support.microsoft.com/default.aspx?scid=kb;en-us;815707 in your mentioning of the FormsCollection?

(points increased to 500 btw)

hey, thats much better that you use framework 2.0, because then you have the possibility of using a generic list which is faster than other collections or lists.
then your frameslist should be something like:
System.Collections.Generic.List<Form> yourFormsList = new System.Collections.Generic.List<Form>();

and you will have to change your else path to:

else
{
     Form formInstance = new Form();
     formInstance.MdiParent = this;
     yourFormsList.Add(formInstance);                  
     clickedBtn.Tag =  yourFormsList.Count-1;
     formInstance.Show();
}
because the add method of a generic list does not return the index of the added item.

the advantage of this implementation is, that you dont have to create a new formobject each time a button is pressed.
if a form has been already created, just use that instance and show it.thats much faster than creating all the objects of the form again. that was the idea of that implementation.
the disadvantage could be, that if you would have 1000 of forms, you will waste your memory. than this implementation would not make any sense.

one sentence to your comment : 'If, say, the user is currently viewing customers and then clicks the 'suppliers' button on the parent form, I don't want the customers form to be closed, it should still be 'under' the supplier form.'
thats managed by your mainform, not by this implementation. this implementation is only good to get already used forms back again, cause you dont have to initialize the form again.

or did i missunderstood something?
just to make clear, i understand your logic.
for example, you have 5 buttons:
then the max created forms are 5 different ones, am i correct?
and if you press for example button 2, the form for button 2 shall be come to front?or shall another one been created for button 2?

I understand now. The speed of re-opening the form is much more noticeable than what little memory is used.
The 'as' cast operator has also been noted, I was not aware of that.
You are indeed correct in your assumptions. No matter how many times any of the 5 buttons are clicked, there will be no more than 5 forms opened, and no two of those are instances of the same form.
So the generic list will save me from creating a FormsCollection : CollectionBase class etc?

I will spend some time working on this later today. Thanks for your time thus far, looks promising.

yes, the generic list is a great new feature in v2.0 of the framework. that are typesave lists  wich are faster than collections.
so if you construct a generic list like List<int> you will only able to store integer in that list. if you construct it like List<Form> you can only store Forms in it.
but now, that i know, that you have a known number of buttons, why dont you store the formhandle you created directly in the tag object of the pressed button? then you dont need any list where you have to store the references.
the click event of the button would look like this then:

void ButtonClickEvent(sender, eventargs)
{
  Button clickedBtn = sender as Button;
  if(clickedBtn.tag != null)                                
 {
    (clickedBtn.Tag as Form).Show();
}
  else                                
  {
     Form formInstance = new Form();
     formInstance.MdiParent = this;
     clickedBtn.Tag =  formInstance;
     formInstance.Show();
  }
}

and to avoid the garbagecollection to dispose the forms, you have to use the forms on closing event , cancel the closing mechanism and hide the form.
private void YourFormsClosingEventHandler(object sender, FormClosingEventArgs e)
{
   e.Cancel = true;
   (sender as Form).Hide();
}

thats a faster mechanism than indexing your form instances via a list of references.
you will have a 1to1 relation of button to forminstance and need less code in your ButtonClickEvent handler, because the form is never really closed so its MdiParent property still references the parent form.

Your last post looks even tidier still, good work. I am trying to implement it now but I must be missing something from your earlier posts :(
This is my OpenChildFormEvent:

private void OpenChildFormEvent(object sender, EventArgs e)
        {
            Button clickedBtn = sender as Button;
            if(clickedBtn.Tag != null)
            {
                (clickedBtn.Tag as Form).Show();
            }  
            else                                
            {
                Form formInstance = new Form();
                formInstance.MdiParent = this;
                clickedBtn.Tag = formInstance;
                formInstance.Show();
            }
        }

I have set all buttons to point to this for their _Click event and deleted the original ShowForm() procedure displayed previously.
I have set the tag to be (for example) "formCustomerList" (without quotes) - is this right? When I try to open a form for the first time, the " (clickedBtn.Tag as Form).Show();" line falls over because I have not created a new instance of the object.

I suggest you to avoid making the things more complicated than they should be. Some notes:
 
  1.   Adding a method for closing event in every form means you make it particular for this MDI parent, is it ok? More, this approach will not remove the forms from MDIChildren collection, so what's the use of creating another FormCollection for the same matter? And this is another good point:
 "If the user has closed the form it is possible that they might not want it again, so what is the point in still keeping a reference to it?" - you should let the user to free memory if this is what he wants!

  2.  Using 'As' operator doesn't produce fewer MSIL instructions than casting. If the cast is possible, the two methods are working just the same (and this is always the case in your code). The difference is, indeed, that using 'As' doesn't throw errors if the conversion is not possible. But there's no difference between the next two sequences, as you checked the types first:

    if(clickedBtn.Tag != null && clickedBtn.Tag is Form )
   {
       (clickedBtn.Tag as Form).Show();
    }

    if(clickedBtn.Tag != null && clickedBtn.Tag is Form )
    {
       ((Form)clickedBtn.Tag).Show();
    }

  Finally, there are some other advantages that the initial approach could offer:
   - A general method that opens a form can be called from many places (is not necessarily tied with Tag property).
   - on a button click event handler you may open different forms based on some conditions;
   - you create an instance of a form the moment you need it, not in mainform constructor.

sumix, maybe there is a misunderstanding in cast: i did not meanthe count of opcode instructions, i mean the opcode instructions themselves.
take a look at this example:

object testClass = new TestClass();

((TestClass)testClass).DoSome(); //(1)
(testClass as TestClass).DoSome(); //(2)

the opcode is:

  .locals init ([0] object testClass)
  IL_0000:  newobj     instance void Sample.TestClass::.ctor()
  IL_0005:  stloc.0
  IL_0006:  ldloc.0 //(1)
  IL_0007:  castclass  Sample.TestClass
  IL_000c:  callvirt   instance void Sample.TestClass::DoSome()
  IL_0011:  ldloc.0 //(2)
  IL_0012:  isinst     Sample.TestClass
  IL_0017:  callvirt   instance void Sample.TestClass::DoSome()
  IL_001c:  ret

the difference between bost casts are the methods castclass vs isinst !
and isinst is much faster because it only checks the reference type but doesn't perform any sort of cast.

and a comment to 1.: he wants that each form is particular.

You are both correct and I intend to use both approaches.
vo1d's approach, using the Tag feature, will be used from the MDI parent since there will be a limited number of buttons (6 or 7 at most) that call it. The speed of re-opening probably outweighs the drop in memory efficiency but I would regard this as bad practice if there were more than, say, 10 forms in this list.
I will be using sumix's approach throughout the rest of the program, where often I will be opening forms from code rather than from a user clicking a button. And even if every form launch was from a button, the Tag is one more feature that has to be managed in the code that can be done without. The few milliseconds extra that would be needed to re-launch a form is nothing compared to the efficiency of managing the potentially unlimited number of child forms that my application will be running.
Thank you both for putting quite a bit of time into this question.
And thanks for your explanations on the 'as' cast operator - also noted.

 
 vo1d, 'isinst' cannot be much faster, a cast is still performed, here's the way 'isinst' works described on MSDN:

"If the class of the object on the top of the stack implements class (if class is an interface) or is a derived class of class (if class is a regular class) then it is cast to type class and the result is pushed on the stack, exactly as though Castclass had been called."

  Regards
 
 

i made a test for both, ten million casts in a 100 loop.
i got very interesting results:
the prefix cast arithmetic mean was : 431 ms / ten million
as operatior cast arithmetic mean was : 393 ms / ten million

if i make a native image of the executable with ngen, the results changed:
the prefix cast arithmetic mean was : 379 ms / ten million
as operatior cast arithmetic mean was : 423 ms / ten million