2012-06-23 25 views
8

Digamos que tenemos estas casillas:¿Cómo hacer que este código sea más SECO?

  • FooCheckBox
  • BarCheckBox
  • BazCheckBox

y estos métodos:

  • Foo
  • Bar
  • Baz

Quiero llamar a cada método solo si la casilla correspondiente está marcada. El código podría tener este aspecto:

void DoWork() 
{ 
    if (FooCheckBox.Checked) 
    { 
     Foo(); 
     Console.WriteLine("Foo was called"); 
    } 

    if (BarCheckBox.Checked) 
    { 
     Bar(); 
     Console.WriteLine("Bar was called"); 
    } 

    if (BazCheckBox.Checked) 
    { 
     Baz(); 
     Console.WriteLine("Baz was called"); 
    } 
} 

Consideremos ahora que en lugar de 3 casillas de verificación y 3 métodos que tener mucho más. ¿Cómo reescribiría el código anterior para hacerlo más DRY?

+0

[Codereview.SE] (http://codereview.stackexchange.com/) es una mejor opción para preguntas de mejora de código como esta. – outis

Respuesta

7

Yo diría que para el caso que presentó, déjelo como está; no desea sobreescribir sin tener una buena razón, ya que puede hacer que una base de código sea menos sostenible. Por supuesto, el contexto importa, y en última instancia, es una decisión.

Dicho esto, así es como abordaría esto. Crea una colección donde cada elemento contiene el control y el delegado de acción. Luego recorra y realice la lógica en cada elemento.

var items = new KeyValuePair<CheckBox, Action>[] { 
    new KeyValuePair<CheckBox,Action>(FooCheckBox, Foo), 
    new KeyValuePair<CheckBox,Action>(BarCheckBox, Bar), 
    new KeyValuePair<CheckBox,Action>(BazCheckBox, Baz) 
}; 

foreach (var item in items) 
{ 
    if (item.Key.Checked) 
    { 
     item.Value.Invoke(); 
     Console.WriteLine("Invoked " + item.Value.Method.Name); 
    } 
} 

O (posiblemente?) Mejor uso de LINQ:

items.Where(item => item.Key.Checked).ToList().ForEach(item => new { 
    item.Value.Invoke(); 
    Console.WriteLine("Invoked " + item.Value.Method.Name); 
}); 
6

Puede usar un diccionario para mantenerse al día con qué acciones se refieren a qué casillas de verificación. Entonces usted podría hacer lo siguiente:

foreach(KeyValuePair<CheckBox, Action> kvp in Dict) 
{ 
    if(kvp.Key.Checked) 
     kvp.Value.Invoke(); 
} 
+0

'Action' no implica argumentos y nulo como tipo de devolución. – Odys

+2

@odyodyodys También lo hacen los 3 métodos propuestos por el OP. –

+0

Correcto. Sus llamadas al Método no implicaban argumentos ni tipos de retorno. Aunque podría reemplazar fácilmente una Función en lugar de la Acción si lo necesitara. – MrWuf

2

Se podría hacer de la siguiente manera:

void DoWork() 
{ 
    Func<Action, string, Tuple<Action, string>> toT = 
     (a, s) => new Tuple<Action, string>(a, s); 

    var map = new Dictionary<CheckBox, Tuple<Action, string>> 
    { 
     {FooCheckBox, toT(Foo, "Foo")}, 
     {BarCheckBox, toT(Bar, "Bar")}, 
     {BazCheckBox, toT(Baz, "Baz")}, 
    }; 

    foreach (var x in map.Keys) 
     if (x.Checked) 
     { 
      map[x].Item1(); 
      Console.WriteLine(map[x].Item2 + " was called"); 
     } 
} 

Pero creo que a veces no siendo muy seco es simplemente De acuerdo.

0

me gustaría crear un Dictionary con <CheckBox, Func> y bucle a través de cada valor:

Dictionary<CheckBox, Func> checkboxes = new Dictionary<CheckBox, Func>(); 
void Init() 
{ 
    checkboxes.Add(FooCheckBox, Foo); 
    checkboxes.Add(BarCheckBox, Bar); 
    checkboxes.Add(BazCheckBox, Baz); 
} 

void DoWork() 
{ 
    foreach (KeyValuePair<CheckBox, Func> checkbox in checkboxes) 
    { 
     if (checkbox.Key.Checked) 
     { 
      checkbox.Value(); 
      Console.WriteLine("{0} was called", checkbox.Text); 
     } 
    } 
} 
4

por simplicidad Me gustaría ir con

void DoWork() 
{ 
    DoIfChecked(FooCheckBox, Foo, "Foo as Called"); 
    DoIfChecked(BarCheckBox, Bar, "Bar as Called"); 
    DoIfChecked(BazCheckBox, Baz, "Baz as Called"); 
} 
void DoIfChecked(CheckBox checkBox, Action action, string message) 
{ 
    if (checkBox.IsChecked) 
    { 
     action(); 
     Console.WriteLine(message); 
    } 
} 

pero se podía hacer algo con la parte del mensaje si es que simple, y podría arrojar algunos errores de verificación dependiendo del contexto local.

Cuestiones relacionadas