What is the best way for multiple and if duplicated refactoring code to extract values ​​from different Controls classes

Question

I have the following code in a WPF application

if (panel != null) { IList listOfValues = new ComparableListOfObjects(); var childControls = panel.GetChildren<Control>(x => x.Visibility == Visibility.Visible); foreach (Control childControl in childControls) { var textBox = childControl as TextBox; if (textBox != null) { listOfValues.Add(textBox.Text); continue; } var comboBox = childControl as ComboBox; if (comboBox != null) { listOfValues.Add(comboBox.SelectedItem); continue; } var datePicker = childControl as DatePicker; if (datePicker != null) { listOfValues.Add(datePicker.SelectedDate.GetValueOrDefault()); continue; } var numericBox = childControl as NumericUpDown; if (numericBox != null) { listOfValues.Add(numericBox.Value); continue; } } 

What is the best approach to refactoring this code by repeating the same logic to extract values ​​from different controls, for example?

  var numericBox = childControl as NumericUpDown; if (numericBox != null) { listOfValues.Add(numericBox.Value); continue; } 

In the same class in another method there is the same code

  private static object GetControlValue(Control control) { if (control == null) throw new ArgumentNullException("control"); var textBox = control as TextBox; if (textBox != null) return textBox.Text; var comboBox = control as ComboBox; if (comboBox != null) return comboBox.SelectedValue; var datePicker = control as DatePicker; if (datePicker != null) return datePicker.SelectedDate.GetValueOrDefault(); var numericUpDown = control as NumericUpDown; if (numericUpDown != null) return numericUpDown.Value; throw new NotSupportedException(); } 

Maybe I should use the strategy development template, but in this case I need to create additional classes for each type of control?

Could you offer me the best solo for this span? Thanks in advance.

Answer 1

The if and switch operations are not bad. Even performing some rudimentary type checks is not necessarily bad, especially when the types used cannot be used polymorphically. When this logic is expressed several times, it is something that is frowned upon because you are repeating yourself and you have several service points for the same changes.

In the source code you are executing

 var blah = obj as Foo; if (blah != null) { someList.Add(blah.Value); } 

And repeat this for several types of controls. But then in your personal method later, you have basically the same logic, expressed as many times.

 var blah = obj as Foo; if (blah != null) return blah.Value; 

The only difference is that in the first fragment you take a value and add it to the list. In the second, you return the value. The first fragment should do away with the type checking logic; it should use the logic already expressed in another method.

 foreach (var control in childControls) { listOfValues.Add(GetControlValue(control)); } 

The idea is not repeated. DRY.

Answer 2

I believe that you are looking for a visitor template . One class for the controller is one way to do this, but to quote the specified article:

Note. A more flexible approach to this pattern is to create a wrapper for a class that implements the interface that defines the accept method. the wrapper contains a link pointing to a CarElement, which can be initialized through the constructor. This approach avoids implementing an interface for each element. [cm. Java Tip article 98 article below]

You may be able to avoid this.

Answer 3

A bit of a hack, but here you can use the delegates and initializers of the collection to eliminate redundancy (you can not use it as it is, but rather an idea).

First create a class as follows:

 // Needs argument validation. Also, extending Dictionary<TKey, TValue> // probably isn't a great idea. public class ByTypeEvaluator : Dictionary<Type, Func<object, object>> { public void Add<T>(Func<T, object> selector) { Add(typeof(T), x => selector((T)x)); } public object Evaluate(object key) { return this[key.GetType()](key); } } 

And then the use will be:

 // Give this variable longer lifetime if you prefer. var map = new ByTypeEvaluator { (ComboBox c) => c.SelectedItem, (TextBox t) => t.Text, (DateTimePicker dtp) => dtp.Value, (NumericUpDown nud) => nud.Value }; Control myControl = ... var myProjection = map.Evaluate(myControl); 

Answer 4

You can do this as a case choice in a universal method, but there is still work in this style:

 public static string GetValue<T>(T obj) where T:Control { switch (obj.GetType().ToString()) { case "TextBox": return (obj as TextBox).Text; break; case "ComboBox": return (obj as ComboBox).SelectedValue.ToString(); break; //..etc... } } 

Answer 5

you can use this approach relying on is :

 private static object GetControlValue(Control control) { if (control == null) throw new ArgumentNullException("control"); if (control is TextBox) return (control as TextBox).Text; if (control is ComboBox) return (control as ComboBox).SelectedValue; ... } 

and

 if (panel != null) { IList listOfValues = new ComparableListOfObjects(); var childControls = panel.GetChildren<Control>(x => x.Visibility == Visibility.Visible); foreach (Control childControl in childControls) { if(childControl is TextBox) { listOfValues.Add((childControl as TextBox).Text); continue; } if(childControl is ComboBox) { listOfValues.Add((childControl as ComboBox).SelectedValue); continue; } ... } } 

continue in the second block is probably not even required, but this requires some testing.