Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Best way to handle redundant code that has repeated logic?

In my form I have four RadioButtons, based on user selection, this code is executed:

private void button1_Click(object sender, EventArgs e)
        {
            listBox1.Items.Clear();
            if (radioButtonName.Checked)
            {
                var Qr = from n in mylist where n.Name == textBoxSearch.Text select new { n.Name, n.Age, n.Occu, n.Gender };
                foreach (var item in Qr)
                {
                    listBox1.Items.Add("Name: " + item.Name + "   " + "  Age: " + item.Age + "   " + "  Occupation: " + item.Occu + "   " + "  Gender: " + item.Gender);
                }
            }
            if (radioButtonAge.Checked)
            {
                var Qr = from n in mylist where n.Age == textBoxSearch.Text select new { n.Name, n.Age, n.Occu, n.Gender };
                foreach (var item in Qr)
                {
                    listBox1.Items.Add("Name: " + item.Name + "   " + "  Age: " + item.Age + "   " + "  Occupation: " + item.Occu + "   " + "  Gender: " + item.Gender);
                }

            }
            if (radioButtonGender.Checked)
            {
                var Qr = from n in mylist where n.Gender == textBoxSearch.Text select new { n.Name, n.Age, n.Occu, n.Gender };
                foreach (var item in Qr)
                {
                    listBox1.Items.Add("Name: " + item.Name + "   " + "  Age: " + item.Age + "   " + "  Occupation: " + item.Occu + "   " + "  Gender: " + item.Gender);
                }
            }
            if (radioButtonOccupation.Checked)
            {
                var Qr = from n in mylist where n.Occu == textBoxSearch.Text select new { n.Name, n.Age, n.Occu, n.Gender };
                foreach (var item in Qr)
                {
                    listBox1.Items.Add("Name: " + item.Name + "   " + "  Age: " + item.Age + "   " + "  Occupation: " + item.Occu + "   " + "  Gender: " + item.Gender);
                }

            }

        }

The code seems very redundant and repeated, but also I can't find a way to handle all 4 RadioButtons in a single line that has only one variable linked to user choice. myList is a List of a class I created that has 4 string properties (Name, Age, Gender, Occu)

like image 462
mshwf Avatar asked Sep 05 '16 19:09

mshwf


People also ask

How do you deal with duplicate codes?

Don't Repeat Yourself (DRY): Using DRY or Do not Repeat Yourself principle, you make sure that you stay away from duplicate code as often as you can. Rather you replace the duplicate code with abstractions or use data normalization. To reduce duplicity in a function, one can use loops and trees.

Why is code duplication not recommended?

It's safe to say that duplicate code makes your code awfully hard to maintain. It makes your codebase unnecessary large and adds extra technical debt. On top of that, writing duplicate code is a waste of time that could have been better spent.

Is it possible to have code with no duplication at all?

There are extreme cases where you prevent code duplication by complicated metaprogramming (not so much an issue for Java) or excessive use of reflection, and in those few cases I'd favor permitting the duplication. This is rare.


2 Answers

wrap everything in a function like this one:

public void foo(RadioButton radioButton, Expression<Func<MyItem, bool>> expression)
    {
        if (radioButton.Checked)
        {
            var Qr = mylist.AsQueryable().Where(expression).Select(x => String.Format("Name: {0}, Age: {1}, Occ: {2}, Gender: {3}", x.Name, x.Age, x.Occu, x.Gender)).ToList();

            foreach (var item in Qr)
            {
                listBox1.Items.Add(item);
            }
        }
    }

    private void button1_Click(object sender, EventArgs e)
    {
        listBox1.Items.Clear();
        foo(radioButtonName, c => c.Gender == textBoxSearch.Text);
        foo(radioButtonAge, c => c.Age == textBoxSearch.Text);
        foo(radioButtonGender, c =>  c.Gender == textBoxSearch.Text);
        foo(radioButtonOccupation, c => c.Occu == textBoxSearch.Text);
    }



public class MyItem
    {
        public String Occu { get; set; }

        public String Age { get; set; }
        public String Name { get; set; }
        public String Gender { get; set; }

    }
like image 194
raven Avatar answered Nov 14 '22 22:11

raven


The only difference is in filter (where) all the other can be combined:

 private void button1_Click(object sender, EventArgs e) {
   var lines = mylist
     .Where(item => radioButtonName.Checked && item.Name == textBoxSearch.Text ||
                    radioButtonAge.Checked && item.Age == textBoxSearch.Text ||
                    radioButtonGender.Checked && item.Gender == textBoxSearch.Text ||
                    radioButtonOccupation.Checked && item.Occu == textBoxSearch.Text)
     .Select(item => string.Format("Name: {0} Age: {1} Occupation: {2} Gender: {3}",
                                    item.Name, item.Age, item.Occu, item.Gender));  

   listBox1.Items.Clear();

   foreach (string line in lines)
     listBox1.Items.Add(line);   
 }  
like image 44
Dmitry Bychenko Avatar answered Nov 14 '22 20:11

Dmitry Bychenko