2010-10-15 25 views
6

Actualmente tengo el siguienteC# ¿este diseño es "Correcto"?

 if (!RunCommand(LogonAsAServiceCommand)) 
      return; 

     if (!ServicesRunningOrStart()) 
      return; 

     if (!ServicesStoppedOrHalt()) 
      return; 

     if (!BashCommand(CreateRuntimeBashCommand)) 
      return; 

     if (!ServicesStoppedOrHalt()) 
      return; 

     if (!BashCommand(BootstrapDataBashCommand)) 
      return; 

     if (!ServicesRunningOrStart()) 
      return; 

habría que ser más limpia de hacer esto? ¿es seguro?

 if (
      (RunCommand(LogonAsAServiceCommand)) 
     && (ServicesRunningOrStart()) 
     && (ServicesStoppedOrHalt()) 
     && (BashCommand(CreateRuntimeBashCommand)) 
     && (ServicesStoppedOrHalt()) 
     && (BashCommand(BootstrapDataBashCommand)) 
     && (ServicesRunningOrStart()) 
     ) 
     { 
       // code after "return statements" here 
     } 
+5

totalmente la misma y una cuestión de estilo, excepto que el primero es más fácil de depurar (y establecer puntos de interrupción). –

+0

Estoy de acuerdo con @Kirk Woll, cuando están optimizados deberían ser lo mismo si el compilador/jitter es lo suficientemente inteligente. – leppie

Respuesta

1

Debe ajustarse a lo que sea más legible y comprensible.

A menos que sea realmente ineficiente.

+0

+1 ¡Esto se une a la expresión "mantenerlo simple"! =) –

3

¿El primer código es un conjunto de sentencias OR?

if ( 
     (!RunCommand(LogonAsAServiceCommand)) 
    || (!ServicesRunningOrStart()) 
    || (!ServicesStoppedOrHalt()) 
    || (!BashCommand(CreateRuntimeBashCommand)) 
    || (!ServicesStoppedOrHalt()) 
    || (!BashCommand(BootstrapDataBashCommand)) 
    || (!ServicesRunningOrStart()) 
+1

Si lo hace como declaraciones O, obtiene la eficiencia de regresar tan pronto como cualquiera de las condiciones es verdadera, en lugar de esperar a ver si todas son verdad. –

+6

Ambos '&&' y '||' están en cortocircuito, por lo que no hay una ventaja real al hacer esto. – Ani

+0

+1 para el rendimiento y para corregir la segunda solución de OP. – BrunoLM

2

Cualquiera está bien. Sin embargo, desde una perspectiva de estilo, sugeriría que, dado que se trata de una serie de comandos, podría usar un List<Action> para mantener estos y hacer que la función funcione como datos.

(new Action[] { func1, func2, .... }).ToList().ForEach(a => a()); 

Por supuesto, ya que las funciones tienen diferentes firmas que pueda tener que ver los múltiples ...

de nuevo, es una cuestión de estilo ....

+1

Este es un buen enfoque, pero aún no es muy amigable. Especialmente cuando llenes la lista con lambda por todos lados. Los métodos normales deberían estar bien, sin embargo. – leppie

+0

de acuerdo en el comentario de depuración. Estoy muy orientado a los datos en mi código, pero lo hace más apto para pruebas unitarias. También tiendo a usar el registro más que la depuración para el análisis en estos días –

+0

Este tipo de cosas se depura mucho mejor en vs2010 :) –

4

Cuando miro el primer enfoque, lo primero que pensé es que la razón por la que el código se ha escrito de esa manera es que las operaciones involucradas probablemente tengan efectos secundarios. De los nombres de los métodos, supongo que lo hacen? Si es así, definitivamente iría con el primer enfoque y no el segundo; Creo que los lectores de su código encontrarán muy confuso ver una expresión de cortocircuito && que se usa para ejecutar condicionalmente los efectos secundarios.

Si no hay efectos secundarios, cualquiera lo hará; ambos son perfectamente legibles. Si usted encuentra que hay varios más condiciones o las circunstancias mismas varían en diferentes escenarios, podría intentar algo como:

Func<bool>[] conditions = ... 
if(conditions.Any(condn => condn())) 
{ 
    ... 
} 

yo no realmente ir con un enfoque de este tipo en su caso, sin embargo.

+0

¿qué quieres decir con "efectos secundarios"? – bevacqua

+0

http://en.wikipedia.org/wiki/Side_effect_(computer_science). Efectivamente, lo que quiero decir en este caso es que la elección de cortocircuito frente a no cortocircuito introduce una diferencia observable y funcional en el estado del programa. – Ani

+2

+1 Por proporcionar una buena regla para decidir entre qué enunciado será más legible. Eric Lippert tiene un post en SO en algún lugar discutiendo esta regla en el contexto del operador ternario, pero parece que no puedo encontrarlo. – Brian

-1

Si está usando el código varias veces que sugeriría lo pones en un método separado, tales como:

public static class HelperClass 
{ 
    public static bool ServicesRunning() 
    { 
    // Your code here... 
    } 
} 

y decir que es cuando se necesita

public class SomeClass 
{ 
    // Constructors etc... 
    public void SomeMethod() 
    { 
    if(HelperClass.ServicesRunning()) 
    { 
     // Your code here... 
    } 
    } 
} 
+0

¿eh, claramente ya estoy haciendo eso? – bevacqua

+0

Bueno, no era exactamente claro que fuera su diseño. Pero si eso es lo que ya estás haciendo, solo puedo decir que estoy de acuerdo con ese diseño. Usando mucho &&/|| if-statements fácilmente se vuelve desordenado (en mi opinión). – Mantisen

Cuestiones relacionadas