Learn how to a build a cloud-first strategyRegister Now

x
?
Solved

Launch a form based on a parameter

Posted on 2006-04-15
17
Medium Priority
?
471 Views
Last Modified: 2010-04-16
I have an MDI application where several different subforms can be launched, but if an instance of a form is already open I want it to be activated instead of being recreated. I have adapted an existing EE solution to this point, and while I could get it working, it would mean quite a lot of code being repeated 7 or 8 times over. I want to be able to pass the form as a parameter, and the function below will check if it is already open.
I chose to make the parameter a string because I can't seem to be able to use "showForm(formCustomerList)" without making a new instance of formCustomerList.  I am under the impression that it is bad practice to create a new instance of something and dispose of it again - is this true?

The code is below. It is the assignment of launchForm that is breaking, VS reports a null value being assigned to it.

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;
            Form[] childForms = this.MdiChildren;   // Get all of the MDI children in an array
            if (childForms.Length == 0)             // no child form is opened
            {
                launchForm = (Form)Activator.CreateInstance(Type.GetType(inFormType));
                launchForm.MdiParent = this;
                launchForm.Show();
            }
            else                                    // child forms are opened
            {
                foreach (Form chform in childForms)
                {
                    if (chform.GetType().Name == inFormType)  // check if one instance of the form is already opened
                    {
                        chform.Activate();
                        break;
                    }
                    else
                    {
                        launchForm =  (Form)Activator.CreateInstance(Type.GetType(inFormType));
                        launchForm.MdiParent = this;
                        launchForm.Show();
                        break;
                    }
                }
            }
        }

How can the above be fixed, or is there is a much better approach to this problem?
0
Comment
Question by:smickell
  • 9
  • 5
  • 3
17 Comments
 
LVL 12

Assisted Solution

by:sumix
sumix earned 600 total points
ID: 16463102

  First of all, the error you get is because GetType method needs a fully qualified class name, including the namespace:
      Type.GetType(namespace+inFormType).
  If you decide to send as parameter for showForm method a string that includes the namespace (which may be a good idea), you'll need to check the existence of an instance using FullName property of Type class (instead of Name):
        chform.GetType().FullName == inFormType ...

  Second, your approach won't give the results you expect because in foreach loop you only check the first element of childForms collection, and if is not of desired type you create a new instance, without checking the rest of the collection. Here's how you can do:

(inFormType parameter includes the namespace)
Form launchForm;
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();
}
0
 
LVL 11

Expert Comment

by:vo1d
ID: 16463624
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
0
 

Author Comment

by:smickell
ID: 16463966
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 :)
0
VIDEO: THE CONCERTO CLOUD FOR HEALTHCARE

Modern healthcare requires a modern cloud. View this brief video to understand how the Concerto Cloud for Healthcare can help your organization.

 
LVL 11

Expert Comment

by:vo1d
ID: 16464236
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
0
 
LVL 11

Expert Comment

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

Author Comment

by:smickell
ID: 16464377
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)
0
 
LVL 11

Expert Comment

by:vo1d
ID: 16464478
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.


0
 
LVL 11

Expert Comment

by:vo1d
ID: 16464557
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?
0
 

Author Comment

by:smickell
ID: 16464666
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.
0
 
LVL 11

Expert Comment

by:vo1d
ID: 16464951
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.
0
 

Author Comment

by:smickell
ID: 16465700
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.
0
 
LVL 11

Accepted Solution

by:
vo1d earned 900 total points
ID: 16467738
you have 5 buttons for 5 forms?
then create all the instances in your mainforms constructor and add the referencers to the buttons tag objects.

so the constructor should look like this:

public YourMainForm()
{
  InitializeComponent();
 
  myFirstForm = new FirstForm();
  firstFormButton.Click +=  new System.EventHandler(OpenChildFormEvent);
  firstFormButton.Tag = myFirstForm;
  //the same for the other buttons and forms;
}

the buttons clickeventhandler will look like this:

private void OpenChildFormEvent(object sender, EventArgs e)
{
  Button clickedBtn = sender as Button;
  if(clickedBtn.Tag != null && clickedBtn.Tag is Form )
  {
    (clickedBtn.Tag as Form).Show();
  }
}

the last eventhandler you will have to add is for the formclosing event of your 5 forms:
that is do avoid that your forms are disposed if they are closed.
private void YourFormsClosingEventHandler(object sender, FormClosingEventArgs e)
{
   e.Cancel = true;
   (sender as Form).Hide();
}



0
 
LVL 12

Expert Comment

by:sumix
ID: 16468209


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.
0
 
LVL 11

Expert Comment

by:vo1d
ID: 16468433
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.
0
 

Author Comment

by:smickell
ID: 16468450
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.
0
 
LVL 12

Expert Comment

by:sumix
ID: 16468503
 
 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
 
 
0
 
LVL 11

Expert Comment

by:vo1d
ID: 16468808
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
0

Featured Post

What does it mean to be "Always On"?

Is your cloud always on? With an Always On cloud you won't have to worry about downtime for maintenance or software application code updates, ensuring that your bottom line isn't affected.

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

Introduction Although it is an old technology, serial ports are still being used by many hardware manufacturers. If you develop applications in C#, Microsoft .NET framework has SerialPort class to communicate with the serial ports.  I needed to…
Exception Handling is in the core of any application that is able to dignify its name. In this article, I'll guide you through the process of writing a DRY (Don't Repeat Yourself) Exception Handling mechanism, using Aspect Oriented Programming.
Exchange organizations may use the Journaling Agent of the Transport Service to archive messages going through Exchange. However, if the Transport Service is integrated with some email content management application (such as an anti-spam), the admin…
Loops Section Overview
Suggested Courses

810 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question