Krdytkyu

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.

I have multiple foreach clauses, multiple lists, multiple conversions of list to datatables. My question is, what can I do to refactor this in order to have clean 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.Checked);

            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);





        }

First step is to look at the repeated code then copy an single into a method.
In this case a could method name would seem to be CreateDataTable.

What are the commonalities?

• A ComboBox that has checked items.


• Get an IQueryable or IEnumerable depending on which is returned by the where clause


• Creates a list of the text value from each checked item


• Creates a DataTable using ToStringDataTable(parameters
  

Since the type in and type out is all the same, this makes it much easier to refactor out into a method. The question then is what input and what output?

Input = ComboBox, output = `DataTable'. This give the method signature needed:

DataTable CreateDataTable(ComboBox cbo) {

}

Now put in those lines that are repeated and change the *Status to cbo and instead of assigning the DataTable just return the result of ToStringDataTable(parameters).

DataTable CreateDataTable(ComboBox cbo) {



    var checkedItems = cbo.CheckBoxItems.Where(x => x.Checked);

    List<string> parameterList = new List<string>();

    foreach (var i in checkedItms) {

        parameter.Add(i.Text);

    }



    return ToStringDataTable(parameters);

}

Now clean up the original method:

public class Refactor {

  private void BtnLoadReport_Click(object sender, EventArgs e)

    {

        var db = new SQLDataMgr();



        var designStatusParameters = CreateDataTable(cboDesignStatus);

        var shopStatusParameters = CreateDataTable(cboShopStatus);

        var customerTypeParameters = CreateDataTable(cboCustomerType);

        var customerParameters = CreateDataTable(cboCustomer);

        var resellerParameters = CreateDataTable(cboReseller);

        var stateParameters = CreateDataTable(cboState);

        var projectManagerParameters = CreateDataTable(cboProjectManager);

        var salesRepresentativeParameters = CreateDataTable(cboSalesRepresentative);

        //Rest of the method

    }   

}

This could be further improved by using some additional LINQ chains if the items returned by the where clause supports them.

Example:

List<String> parameters = cbo.CheckBoxItems.Where(x => x.Checked).Select(x => x.Text).ToList();

which would simplifying the method even further:

DataTable CreateDataTable(ComboBox cbo) {



    List<string> parameterList = List<String> parameters = cbo.CheckBoxItems.Where(x => x.Checked).Select(x => x.Text).ToList();



    return ToStringDataTable(parameters);

}