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);
}
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();
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.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With