switch sirve que para for else c++ c if-statement code-review

for - if else c++ para que sirve



Anted redundated ''if'' condiciones (9)

¿Hay alguna forma mejor (o más limpia) de escribir el siguiente código?

if(conditionX) { if(condition1) { // code X1 } else if(condition2) { // code X2 } } else if(conditionY) { if(condition1) { // code Y1 } else if(condition2) { // code Y2 } }

Tengo algunas condiciones más, pero supongo que entiendes el punto.


  1. La mejor manera aquí sería usar polimorfismo (solo si los trozos de código son enormes)

  2. Si son pequeños fragmentos de código, la creación de clases obviamente sería una exageración.

    Por lo tanto, si hay similitud en todos los códigos, sugeriría una tarea aparentemente fácil pero realmente difícil.

    • Intenta parametrizarlos tanto como puedas.
    • Cree una función que los tome y llámelos en las condiciones
    • Ahora el código estaría en bloques funcionales y "limpiador"

Siempre es difícil crear cosas simples.

if (conditionX) { method(parameterX); else if (conditionY) { method(parameterY); }

dónde

void method(ParameterType e) { if (condition 1) { // Code in terms of parameter e } else if (condition2) { // Code in terms of parameter e } }

La condición que usted puede parametrizar debe mantenerse afuera.

Espero que esto ayude.


Creo que de esta manera puede ser otra forma de resolver tu código.

enum ConditionParentType { CONDITION_NONE = 0, CONDITION_X, CONDITION_Y, }; enum ConditionChildType { CONDITION_0 = 0, CONDITION_1, CONDITION_2, }; class ConditionHandler { public: explicit ConditionHandler(ConditionParentType p_type, ConditionChildType c_type) : p_type_(p_type), c_type_(c_type) {}; void DoAction() { if(child_type == CONDITION_1) { } else if(child_type == CONDITION_2) { } else { //error } } private: const ConditionParentType p_type_; const ConditionChildType c_type_; }; int main(int argc, char *argv[]) { ConditionParentType parent_type = GetParentType(); ConditionChildType child_type = GetChildType(); ConditionHandler handler(parent_type, child_type); handler.DoAction(); getchar(); return 0; }


Estoy bastante sorprendido por las otras respuestas sugeridas, que en su mayoría son incorrectas si:

  • Las dos condiciones repetidas condition1 o condition2 son complejas, en cuyo caso DRY entra en juego, o
  • Cualquiera de las cuatro condiciones tiene efectos secundarios, o
  • Cualquiera de las condiciones es lenta (por ejemplo, encontrar el mínimo de una matriz grande, o leer un archivo), o
  • Se necesita un cortocircuito booleano, como en: if (p == 0) {...} else if (p->foo == 42) {...} .

Si ninguno de estos se mantiene, como es el caso el 99.42% del tiempo, entonces abandone el código tal como está. O, como una variación menor, cámbiela para que la anidación (es decir, la sangría) sea solo un nivel, no dos.

De lo contrario, deberá usar variables temporales de la siguiente manera

const bool tstX = (conditionX); const bool tstY = tstX || (conditionY); const bool tst1 = tstY && (condition1); const bool tst2 = tstY && !tst1 && (condition2);


Hay cuatro enfoques para este problema, ninguno de los cuales es universal:

  1. Deje todo como está : no hay mucha duplicación de código aquí. Si el cálculo de condition1 y condition2 es complicado, cómprálos por adelantado y guárdelos en variables bool
  2. Haga que conditionX y conditionY produzcan un resultado que le permita unificar condition1 y condition2 - Esto no siempre es posible, pero en algunas situaciones podría preparar una variable que unifica las actividades tomadas en las dos ramas, por ejemplo, usando un puntero de función o un lambda .
  3. Coloque la lógica de procesamiento en subclases con funciones virtuales para eliminar la lógica condicional . Esto solo es posible cuando el diseño inicial perdió la oportunidad de crear una subclase. Esencialmente, este enfoque empuja la decisión sobre conditionX / conditionY a un lugar donde se crea una subclase, y luego "reutiliza" esa decisión más tarde llamando a una anulación adecuada de una función virtual en la interfaz.
  4. Cree una combinación numérica que represente las tres condiciones y convierta para switch : este truco unifica los condicionales, reduciendo el anidamiento.

Aquí hay un ejemplo del último enfoque:

int caseNumber = ((conditionX?1:0) << 3) | ((conditionY?1:0) << 2) | ((condition2?1:0) << 1) | ((condition1?1:0) << 0); switch (caseNumber) { case 0x09: case 0x0D: case 0x0F: // code X1 break; case 0x0A: case 0x0E: // code X2 break; case 0x05: case 0x07: // code Y1 break; case 0x06: // code Y2 break; }


Proporcionaría la decisión dentro del primero si como parámetro de una función separada, que luego decide qué código ejecutar, como:

if(conditionX) { Method1(Condition Parameters) } else if(conditionY) { Method1(Condition Parameters) }

Otra forma sería proporcionar toda la información necesaria a un método de decisión (matriz), este método devuelve un entero que se usa en una instrucción switch para decidir qué código ejecutar. De esta forma, se separa la lógica de la desición que la hace legible y fácil de probar si es necesario:

DecisionMatrix(conditionX, conditionY, condition1, condition2) { // return a value according to the conditions for Example: // CoditionX + Condition1 => return 1 // CoditionX + Condition2 => return 2 // CoditionY + Condition1 => return 3 // CoditionY + Condition2 => return 4 } switch(DecisionMatrix) { case 1: //run code X1 break; case 2: //run code X2 break; case 3: //run code Y1 break; case 4: //run code Y2 break; }


Puede implementar una máquina de estados en su lugar:

#define COMBINATION(a,b,c,d) (((a)<<3)|((b)<<2)|((c)<<1)|((d)<<0)) switch (COMBINATION(conditionX,conditionY,condition1,condition2)) { case COMBINATION(0,0,0,0): break; case COMBINATION(0,0,0,1): break; case COMBINATION(0,0,1,0): break; case COMBINATION(0,0,1,1): break; case COMBINATION(0,1,0,0): break; case COMBINATION(0,1,0,1): CodeY2(); break; case COMBINATION(0,1,1,0): CodeY1(); break; case COMBINATION(0,1,1,1): CodeY1(); break; case COMBINATION(1,0,0,0): break; case COMBINATION(1,0,0,1): CodeX2(); break; case COMBINATION(1,0,1,0): CodeX1(); break; case COMBINATION(1,0,1,1): CodeX1(); break; case COMBINATION(1,1,0,0): break; case COMBINATION(1,1,0,1): CodeX2(); break; case COMBINATION(1,1,1,0): CodeX1(); break; case COMBINATION(1,1,1,1): CodeX1(); break; }

Esto incluye solo una operación de sucursal, por lo que es posiblemente un poco más eficiente (aunque también incluye un cálculo de tiempo de ejecución adicional (en la línea del switch )).

En cuanto a ser más limpio, supongo que es una cuestión de perspectiva, pero la plantilla anterior también le brinda una forma conveniente de detectar todas las ramas no administradas dentro de su código.

Tenga en cuenta que si alguna de las variables de condición puede tener un valor distinto de 1 o 0, entonces debe:

#define COMBINATION(a,b,c,d) (((a)?8:0)|((b)?4:0)|((c)?2:0)|((d)?1:0))

Actualización (atribuida a @Jonathan Wakely en uno de los comentarios a continuación):

Si está utilizando C ++ 11, puede reemplazar la macro COMBINATION con una función constexpr :

constexpr int COMBINATION(bool a,bool b,bool c,bool d) { return ((int)a<<3) | ((int)b<<2) | ((int)c<<1) | ((int)d<<0); }


Si la combinación de condiciones significa algo, escribiría un conjunto de métodos simples que devuelvan valores booleanos. Usted terminaría con algo como:

if (first-condition(conditionX, condition1)) { // code X1 } else if (first-condition(conditionX, condition2)) { // code X2 } else if (third-condition(conditionY, condition1)) { // code Y1 } else if (fourth-condition(conditionY, condition2)) { // code Y2 }

Los nombres de los métodos describen las condiciones. No se preocupe, los métodos solo se invocan una vez (el compilador probablemente los alineará de todos modos), lo importante es que su código se documente automáticamente.


Si su preocupación es con el código limpio en términos de ver la fuente, mi consejo sería segregar los bloques en sus propias secciones, algo así como:

if (conditionX) processConditionX(); else if (conditionY) processConditionY();

y así.

Luego, en las subfunciones, colocas la "carne":

void processConditionX (void) { if(condition1) { // code X1 } else if(condition2) { // code X2 } }

Puedes modificarlo para que entre y devuelva los parámetros según sea necesario, y haré que las condiciones y los nombres de las funciones sean un poco más descriptivos, aunque supongo que son solo ejemplos aquí.


el código original no se ve mal. Dependiendo del caso específico, puede o no ser más legible para hacer algo como:

if(conditionX and condition1) { // code X1 } else if(conditionX and condition2) { // code X2 } else if(conditionY and condition1) { // code Y1 } else if(conditionY and condition2) // code Y2 }