Krdytkyu

I have a windows form in C# project that keeps some information. I created a bunch of textbox and combobox dynamically, depend upon user input.

Consider, I have a list of 10 combobox whose values will be selected by user.

The combobox values are:

1. Finding


2. No Finding


3. Incomplete


4. Skipped.

Now I have a single final combobox which contains the following values.

1. Finding


2. No Finding


3. Incomplete


4. Skipped Reviewed


5. Skipped Not reviewed

The final combobox should be populated depend upon the following logic

1. If all values are "Finding", then the the final combobox should be "Finding"


2. If all values are "No_Finding", then the the final combobox should be "No_Finding"


3. If all values are "InComplete", then the the final combobox should be "InComplete"



4. If all values are "Skipped", then the the final combobox should be "Skipped Not Reviewed".




5. If any value is "Finding", then the final combobox should be "Finding".



6. If any value is "Incomplete", then the final combobox should be "Incomplete".


7. If any value is "Skipped", then the final combobox should be "Skipped Reviewed".

With respect to the above logic conditions I have written the following that works fine.

After writing the code I feel it's not cleaner and not an easily understandable solution.

What I need now whether there is any way to refactor the following lines of code.

 public void selectfinalComboxValue()

        {

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

//txtBoxValLines is an user input 

            for (int i = 0; i < int.Parse(txtboxvalLines.Text); i++)

            {

                string cmbboxValue = ((ComboBox)panel1.Controls["Add_combobox" + (i).ToString()]).Text;

                list_of_combobox.Add(cmbboxValue);



            }

            List<string> distinct = list_of_combobox.Distinct().ToList();



            if (distinct.Any(str => str.Contains("InComplete")))

            {

                cmbFinalStatus.SelectedIndex = 2;

                cmbFinalStatus.Enabled = false;

            }

            else if (distinct.Any(str => str.Equals("Finding")))

            {

                cmbFinalStatus.SelectedIndex = 0;

                cmbFinalStatus.Enabled = false;

            }

            else

            {

                cmbFinalStatus.SelectedIndex = -1;

                cmbFinalStatus.Enabled = true;

            }



            if (distinct.Count().ToString() == "1")

            {



                if (distinct.Any(str => str.Equals("Skipped")))

                {

                    cmbFinalStatus.SelectedIndex = 4;

                    cmbFinalStatus.Enabled = false;

                }

                else if (distinct.Any(str => str.Equals("No Finding")))

                {

                    cmbFinalStatus.SelectedIndex = 1;

                    cmbFinalStatus.Enabled = false;

                }

            }

            else {

                if (distinct.Any(str => str.Contains("Skipped")))

                {

                    cmbFinalStatus.SelectedIndex = 3;

                    cmbFinalStatus.Enabled = false;

                }



            }            

        }

I won't give you the full response, because your code lack of context (we didn't know nothing about cmbFinalStatus...).

Without using distinct

result value is set if case case 1,2,3,4 match.

List<string> all_of = new List<string>(){"Skipped", "Incomplete", "No_Finding", "Finding"};

var result = all_of.FirstOrDefault(lhs => list_of_combobox.All(rhs => rhs.Equals(lhs)));

if (

result value is set if case 5,6,7 match.

List<string> first_of = new List<string>(){"Skipped", "Incomplete", "Finding"};

var result = first_of.FirstOrDefault(lhs => list_of_combobox.Any(rhs => rhs.Equals(lhs)));

Edit: Putting all together, we got:

  // for... populating list_of_combobox

  // ...



  var str = new List<string>(){"Finding", "No Finding", "InComplete", "Skipped"}

                .Where(lhs => list_of_combobox.All(rhs => lhs.Equals(rhs)))

                .FirstOrDefault() ?? string.Empty;



  if (str.Length != 0)

  {

      if (str.Equals("Skipped")) str = "Skipped Not Reviewed";

  }

  else

  {

      str = new List<string>(){"Finding", "InComplete", "Skipped"}

                .Where(lhs => list_of_combobox.Any(rhs => lhs.Equals(rhs)))

                .FirstOrDefault() ?? string.Empty;      



      if (str.Equals("Skipped")) str = "Skipped Reviewed";

   }



  cmbFinalStatus.SelectedIndex = new List<string>(){"Finding", "No Finding", "InComplete", "Skipped Reviewed", "Skipped Not Reviewed"}

                                     .IndexOf(str);



  cmbFinalStatus.Enabled = (cmbFinalStatus.SelectedIndex < 0);

I think you might be able to eliminate creating the distinct list and the nested ifs by first checking for the All() condtions, then later check for Any()

    var items = list_of_combobox.ToList();



    if (items.All(str => str.Equals("Finding")))

    {

       // assign final comboxbox 

    }

    else  if (items.Any(str => str.Equals("Finding")))

Also, you might want to try creating an enum of the possible values, adding these values to the ComboBox, then getting/setting the SelectedItem property rather than dealing with indexes.