Reusing a method between classes-CodePudding

A simple/beginner question on code resuse in a basic OOP program recording the sale of motor vehicle tyres.

The tyres are stored in an array:

public Tyre[] tyres = new Tyre[5];

I have two forms.

Form1 End user simply uses a combo/lookup of current stock items (tyres) to select item.

public partial class Form1 : Form
    {
        Fitting fitting;
        public Tyre[] tyres = new Tyre[5];
        Tyre currentTyre;
        public Form1()
        {
            InitializeComponent();
            
            //populate array with tyres
            tyres[0] = new Tyre("155/80S13", 56.00m, 10);
            tyres[1] = new Tyre("165/70P15", 42.00m, 10);
            tyres[2] = new Tyre("195/70S13", 46.00m, 10);
            tyres[3] = new Tyre("158/90S19", 70.00m, 10);
            tyres[4] = new Tyre("185/66R13", 66.00m, 10);    
        }
        
        // search through array to find current selected tyre
        public  Tyre findTyre(string tyretype)
        {
            for (int i = 0; i < tyres.Length; i  )
            {
                if (tyretype == tyres[i].Type)
                    return tyres[i];
            }
            return null;
        }

 private void cmbTyreType_SelectedIndexChanged(object sender, EventArgs e)
        {
            currentTyre = findTyre(cmbTyreType.Text);
            lblPrice.Text = currentTyre.Price.ToString();
            lblStockQ.Text = currentTyre.StockQty.ToString();
        }

The findTyre() method is defined as:

public  Tyre findTyre(string tyretype)
        {
            for (int i = 0; i < tyres.Length; i  )
            {
                if (tyretype == tyres[i].Type)
                    return tyres[i];
            }
            return null;
        }

Form 2 (AddStock) On this form, the end user currently uses a similar combo/lookup again to view the range of tyres (just like on Form1)

public partial class AddStock : Form
    {
        Form1 frm2;
        Tyre currentTyre;
       
        public AddStock(Form1 frm)
        {
            frm2 = frm;
            InitializeComponent();
            for (int i = 0; i< frm.tyres.Length ; i  )
            {
                cmbTyreType.Items.Add(frm.tyres[i].Type);
            }
        }

        public Tyre findTyre(string tyretype )
        {
            for (int i = 0; i < frm2.tyres.Length; i  )
            {
                if (tyretype == frm2.tyres[i].Type)
                    return frm2.tyres[i];
            }
            return null;
        }

        

        private void cmbTyreType_SelectedIndexChanged(object sender, EventArgs e)
        {
            currentTyre = findTyre(cmbTyreType.Text);
            lblCurrentQ.Text = currentTyre.StockQty.ToString();
        }

My concern is that I've had to re-define findTyre() again, eventhough it was already defined in Form1. My hope was (perhaps ill-informed) that i could re-use the findTyre() method from Form1, but Visual Studio is preventing me.

Could the reason be that the findTyre() method is bound to instances which render it inaccessible outside the class?

CodePudding user response：

Create a class that manages your tyres:

public class Tyres
{
   private Tyre[] tyres = new Tyre[5];

   public Tyre this[int i]
   {
      get { return tyres[i]; }
      set { tyres[i] = value; }
   }

   public Tyre findTyre(string tyretype )
   {
       for (int i = 0; i < frm2.tyres.Length; i  )
       {
          if (tyretype == frm2.tyres[i].Type)
              return frm2.tyres[i];
       }
       return null;
   }
}

Instead of passing the array between forms, pass an instance of this class. This class can also hold even more specific methods, which do operations on your tyres. This is one fundamental of OOP: keeping the data and the methods acting on that data togehter (in a class). This also helps you separating your logic from your GUI. Imagine you would like to change your winforms application to a console or a web application. In that case, you can reuse the tyres class. Searching for a tyre has nothing to do with a GUI, that's why it doesn't belong into a form class.

CodePudding user response：

In AddStock can't you just use the findTyre method from Form1?

currentTyre = frm2.findTyre(cmbTyreType.Text);

CodePudding user response：

Seems you can just call findTyre from Form1

public Tyre findTyre(string tyretype)
=> frm2.findTyre(tyretype)

CodePudding user response：

Why is the findTyre method inside the forms class and not inside the Tyres class? In your case i would just move the method inside the Tyre object class make it static and add a new parameter to the method.

It is never good to have duplicate methods inside your program. Reason being if that method changes functionaly you will have to change it everywhere in your whole program instead of one. Also always try to bind you object specific code to your object class. So you know if you need a method that is linked to tyres you just need to look into the tyres class and not go through all your classes trying to find it.

I would recommend you also read the following article, it is an explanation to what separation of concerns is: https://www.castsoftware.com/blog/how-to-implement-design-pattern-separation-of-concerns

     public static Tyre findTyre(string tyretype, Tyres[] Tyres )
    for (int i = 0; i < tyres.Length; i  )
        {
            if (tyretype == tyres[i].Type)
                return tyres[i];
        }
        return null;
    }

I would also recommend you implementering the code @SomeBody provided in the answer below. Will make your code cleaner and more sustainable.