Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Creating method to use one foreach instead multiple

Tags:

c#

winforms

I'm working with c#, and I have working code where I repeated same code every line, and that's because I'm creating list, conversions , extracting data etc.

So I have multiple foreach clauses,multiple lists, multiple conversions of list to datatables. My question is, what can I do to re-factorize this in order to have clean code?

Code:

private void BtnLoadReport_Click(object sender, EventArgs e)
{
   var db = new SQLDataMgr();

   List<string> DesignStatusList = new List<string>();
   List<string> ShopStatusList = new List<string>();
   List<string> CustomerTypeList = new List<string>();
   List<string> CustomerList = new List<string>();
   List<string> ResellerList = new List<string>();
   List<string> StateList = new List<string>();
   List<string> ProjectManagerList = new List<string>();
   List<string> SalesRepresentativeList = new List<string>();

   var checkedDesignStatus = cboDesignStatus.CheckBoxItems.Where(x => x.Checked);
   var checkedShopStatus = cboShopStatus.CheckBoxItems.Where(x => x.Checked);
   var checkedCustomerType = cboShopStatus.CheckBoxItems.Where(x => x.Check       
   var checkedCustomer = cboShopStatus.CheckBoxItems.Where(x => x.Checked);
   var checkedReseller = cboShopStatus.CheckBoxItems.Where(x => x.Checked);
   var checkedState = cboShopStatus.CheckBoxItems.Where(x => x.Checked);
   var checkedProjectManager = cboShopStatus.CheckBoxItems.Where(x => x.Checked);
   var checkedSalesRepresentative = cboShopStatus.CheckBoxItems.Where(x => x.Checked);

   foreach (var i in checkedDesignStatus)
   {
      DesignStatusList.Add(i.Text);
   }
   foreach (var i in checkedShopStatus)
   {
      ShopStatusList.Add(i.Text);
   }
   foreach (var i in checkedCustomerType)
   {
      CustomerTypeList.Add(i.Text);
   }
   foreach (var i in checkedCustomer)
   {
      CustomerList.Add(i.Text);
   }
   foreach (var i in checkedReseller)
   {
      ResellerList.Add(i.Text);
   }
   foreach (var i in checkedState)
   {
      StateList.Add(i.Text);
   }
   foreach (var i in checkedProjectManager)
   {
      ProjectManagerList.Add(i.Text);
   }
   foreach (var i in checkedSalesRepresentative)
   {
      SalesRepresentativeList.Add(i.Text);
   }
   DataTable designStatusParameters = ToStringDataTable(DesignStatusList);
   DataTable shopStatusParameters = ToStringDataTable(ShopStatusList);
   DataTable customerTypeParameters = ToStringDataTable(CustomerTypeList);
   DataTable customerParameters = ToStringDataTable(CustomerList);
   DataTable resellerParameters = ToStringDataTable(ResellerList);
   DataTable stateParameters = ToStringDataTable(StateList);
   DataTable projectManagerParameters = ToStringDataTable(ProjectManagerList);
   DataTable salesRepresentativerParameters = ToStringDataTable(SalesRepresentativeList);
}
like image 413
Jonathan Avatar asked Dec 06 '22 10:12

Jonathan


2 Answers

Change ToStringDataTable to an Extension Method. Then:

var designStatusParameters = cboDesignStatus.CheckBoxItems.Where(x => x.Checked)
    .Select(i => i.Text).ToList().ToStringDataTable();

You can even write the extension method for your control in a way that performs filtering, projection and converting to data table for you then you can have:

var designStatusParameters = cboDesignStatus.ToStringDataTable();
like image 114
Reza Aghaei Avatar answered Dec 25 '22 15:12

Reza Aghaei


You can cut out all the foreach loops by using Linq's Select():

For example:

var DesignStatusList =
    cboDesignStatus.CheckBoxItems
        .Where(x => x.Checked)
        .Select(i => i.Text)
        .ToList();

That will leave you with a List<string> containing all the Text properties from the checked CheckBoxes.

You could even skip declaring the list and combine that with your DataTable creation lines:

var designStatusParameters = ToStringDataTable(
    cboDesignStatus.CheckBoxItems
                .Where(x => x.Checked)
                .Select(i => i.Text)
                .ToList());

I would suggest putting this in a method of its own rather than repeating this over and over for each group of checkboxes.

Just keep in mind that less lines of code doesn't mean faster performance. It still has to iterate through the lists to find the right values. But it is much more readable than being hit in the face with a wall of repetitive code.

like image 22
Gabriel Luci Avatar answered Dec 25 '22 15:12

Gabriel Luci