# calling a function from another form C#



## Sisaroth

You are making a new Form1 object which you then add the data to. You should instead use the Form1 object that you actually displayed.

A not so clean way to do that would be:

Code:



Code:


Form2
{
        private Form1 f1;

        public Form2(Form1 f){
                f1 = f;
        }

        private void SaveButton_Click(object sender, EventArgs e)
        {
            f1.AddData("cat", "dog", "car", "table", "mouse", "whatever");
            this.Close();
        }
}

And when you make the Form2 object you do it with new Form2(form1);

It would be easier to help you if you also post the code where you make Form1 and Form2.


----------



## Fantasy

ok. i'm sorry but i still don't understand how this works. I'm getting a null exception on *f1* varible. It was never assigned to anything. do i have to call Form2 function or what exactly?


----------



## tompsonn

Pass the instance of Form1 to the Form2 constructor:

Code:



Code:


public partial class Form1 : Form
{
        public Form1()
        {
            InitializeComponent();
        }

        public void AddData(string A, string B, string C, string D, string E, string F)
        {
            ListViewItem lvi = new ListViewItem(A);

            lvi.SubItems.Add(B);
            lvi.SubItems.Add(C);
            lvi.SubItems.Add(D);
            lvi.SubItems.Add(E);
            lvi.SubItems.Add(F);

            listView1.Items.Add(lvi);
        }

        private void EditButton_Click(object sender, EventArgs e)
        {
        }

        private void AddButton_Click(object sender, EventArgs e)
        {
            Form2 f = new Form2(this); /* pass this instance to new Form2 object */
            f.Show();
        }
}

public partial class Form2 : Form
{
        private readonly Form1 _form1; /* storage for Form1 instance */

        public Form2( Form1 form1 )
        {
            InitializeComponent();
            this._form1 = form1;
        }

        private void SaveButton_Click(object sender, EventArgs e)
        {
            this._form1.AddData("cat", "dog", "car", "table", "mouse", "whatever");
            this.Close();
        }
}


----------



## Fantasy

oo wow. thank you so much. How did i not think of that









I guess i'm a little rusty after leaving programming for three months.


----------



## tompsonn

Quote:


> Originally Posted by *Fantasy*
> 
> oo wow. thank you so much. How did i not think of that
> 
> 
> 
> 
> 
> 
> 
> 
> 
> I guess i'm a little rusty after leaving programming for three months.












Sisaroth was suggesting the same thing, BTW.


----------



## Fantasy

Quote:


> Originally Posted by *tompsonn*
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> Sisaroth was suggesting the same thing, BTW.


yah. i didn't get it at first. now i get it.

man I remember solving the same problem in college. but eeeh. i don't know.

anyways, thank you both guys


----------



## 3930K

A better way to do this would be to pass a delegate to the constructor:

Code:



Code:


public partial class Form1 : Form
{
        public Form1()
        {
            InitializeComponent();
        }

        public void AddData(string A, string B, string C, string D, string E, string F)
        {
            ListViewItem lvi = new ListViewItem(A);

            lvi.SubItems.Add(B);
            lvi.SubItems.Add(C);
            lvi.SubItems.Add(D);
            lvi.SubItems.Add(E);
            lvi.SubItems.Add(F);

            listView1.Items.Add(lvi);
        }

        private void EditButton_Click(object sender, EventArgs e)
        {
        }

        private void AddButton_Click(object sender, EventArgs e)
        {
            Form2 f = new Form2(AddData); //pass the delegate to Form2
            f.Show();
        }
}

public partial class Form2 : Form
{
        private Action<string, string, string, string, string, string> AddItems;
        public Form2(Action<string, string, string, string, string, string> itemsAction)
        {
            AddItems = itemsAction; //store it
            InitializeComponent();
        }

        private void SaveButton_Click(object sender, EventArgs e)
        {
            AddItems("cat", "dog", "car", "table", "mouse", "whatever"); //call it
            this.Close();
        }
}

This decouples Form2 from Form1. That is good.


----------



## tompsonn

Quote:


> Originally Posted by *3930K*
> 
> A better way to do this would be to pass a delegate to the constructor:
> 
> This decouples Form2 from Form1. That is good.


Not really. Perhaps the two forms are related and should be coupled? Someone looking at the code can see quickly with the first way that the method call resides in Form1. With a delegate you have no way of knowing where that method call actually is at an easy glance.

Also what happens if you need to call multiple methods on Form1 in the future? Do you simply just pass the object instance, or pass _n_ delegates to the constructor...Ouch, that becomes hard to manage. If you're going to do that, you've basically just re-invented the wheel (which would be storing pointers to functions [delegate] inside a structure [class]). Oh wait, C# already does this!









Furthermore, let's look at what happens if the method signature needs to change. With the delegate way, we'd need to change the signature in 4 places (first on the method itself, second in the constructor parameter, third on the property storing the delegate, and fourth on the delegate invocation). Instance method? Change twice - first on the method implementation, second on the invocation.

It just screams manageability nightmare for production code that you really need to look at what might you do down the track.

Ideally you'd want to be using an interface instead. That keeps things separated and abstracted and still retains ease of management.


----------



## 3930K

Quote:


> Originally Posted by *tompsonn*
> 
> Quote:
> 
> 
> 
> Originally Posted by *3930K*
> 
> A better way to do this would be to pass a delegate to the constructor:
> 
> This decouples Form2 from Form1. That is good.
> 
> 
> 
> Not really. Perhaps the two forms are related and should be coupled? Someone looking at the code can see quickly with the first way that the method call resides in Form1. With a delegate you have no way of knowing where that method call actually is at an easy glance.
> 
> Also what happens if you need to call multiple methods on Form1 in the future? Do you simply just pass the object instance, or pass _n_ delegates to the constructor...Ouch, that becomes hard to manage. If you're going to do that, you've basically just re-invented the wheel (which would be storing pointers to functions [delegate] inside a structure [class]). Oh wait, C# already does this!
> 
> 
> 
> 
> 
> 
> 
> 
> 
> Furthermore, let's look at what happens if the method signature needs to change. With the delegate way, we'd need to change the signature in 4 places (first on the method itself, second in the constructor parameter, third on the property storing the delegate, and fourth on the delegate invocation). Instance method? Change twice - first on the method implementation, second on the invocation.
> 
> It just screams manageability nightmare for production code that you really need to look at what might you do down the track.
> 
> Ideally you'd want to be using an interface instead. That keeps things separated and abstracted and still retains ease of management.
Click to expand...

You have a point with the interface. And yes, it is annoying, but it's what best practices says. Best practices are annoying.


----------



## tompsonn

Quote:


> Originally Posted by *3930K*
> 
> You have a point with the interface. And yes, it is annoying, but it's what best practices says. Best practices are annoying.


Where does it say its best practice to pass delegates around like that!? (Taking into account future maintainability and expansion...)


----------



## 3930K

Quote:


> Originally Posted by *tompsonn*
> 
> Quote:
> 
> 
> 
> Originally Posted by *3930K*
> 
> You have a point with the interface. And yes, it is annoying, but it's what best practices says. Best practices are annoying.
> 
> 
> 
> Where does it say its best practice to pass delegates around like that!? (Taking into account future maintainability and expansion...)
Click to expand...

Demeter's law of Decoupling Everything™


----------



## tompsonn

Quote:


> Originally Posted by *3930K*
> 
> Demeter's law of Decoupling Everything™


Load of crap sir









Well not really, its just that it shouldn't be a law and that it should probably be called an idea instead. In fact the law is tersely defined as "only talk to your immediate friends", ergo even the first example complies with this law anyway!


----------



## 3930K

Quote:


> Originally Posted by *tompsonn*
> 
> Quote:
> 
> 
> 
> Originally Posted by *3930K*
> 
> Demeter's law of Decoupling Everything™
> 
> 
> 
> Load of crap sir
> 
> 
> 
> 
> 
> 
> 
> 
> 
> Well not really, its just that it shouldn't be a law and that it should probably be called an idea instead. In fact the law is tersely defined as "only talk to your immediate friends", ergo even the first example complies with this law anyway!
Click to expand...

It should. But a butnh of people that stick to these so-called laws would reject this (Form1 has nothing to do with Form2! (╯°□°）╯︵ ┻━┻)


----------



## tompsonn

Quote:


> Originally Posted by *3930K*
> 
> It should. But a butnh of people that stick to these so-called laws would reject this (Form1 has nothing to do with Form2! (╯°□°）╯︵ ┻━┻)


LOL.


----------

