c++ - ejemplo - Optimizar el operador ternario.

operador ternario python 3

Question

Me encontré con este código escrito por otra persona. ¿Se recomienda o se usa comúnmente este uso del operador condicional? Siento que es menos mantenible, ¿o soy solo yo? ¿Hay alguna forma alternativa de escribir esto?

  exp_rsp_status =  req.security_violation ? (dis_prot_viol_rsp && is_mstr) ? 
                    uvc_pkg::MRSP_OKAY : uvc_pkg::MRSP_PROTVIOL : req.slv_req.size()  ?
                    ((is_mst_abort_rsp && dis_mst_abort_rsp) ||
                    ((req.slv_req[0].get_rsp_status()==uvc_pkg::MRSP_PROTVIOL) && dis_prot_viol_rsp) ||
                    (is_mst_abort_rsp && req.is_pci_config_req() && dis_pcicfg_mst_abort_rsp)) ?
                    uvc_pkg::MRSP_OKAY : req.slv_req[0].get_rsp_status() : uvc_pkg::MRSP_OKAY;

Answer 1

Común o recomendado? No.

Hice algo similar, pero tenía mis razones:

1. Fue un argumento en una función C de terceros.
2. No estaba bien versado en C ++ moderno en ese momento.
3. Comenté y formateé la mierda de ella porque sabía que ALGUIEN además de mí la iba a leer ... o necesitaba saber qué estaba haciendo años después.

4. Era el CÓDIGO DE DEPURACIÓN que nunca iba a ser lanzado.

   textprintf_ex(gw->GetBackBuffer(), font, 0, 16, WHITE, -1, "BUTTON: %s",
                          //If...                        Then Display...
                     (ButtonClicked(Buttons[STOP])    ?  "STOP"
                   : (ButtonClicked(Buttons[AUTO])    ?  "AUTO" 
                   : (ButtonClicked(Buttons[TICK])    ?  "TICK"
                   : (ButtonClicked(Buttons[BLOCK])   ?  "BLOCK"
                   : (ButtonClicked(Buttons[BOAT])    ?  "BOAT"
                   : (ButtonClicked(Buttons[BLINKER]) ?  "BLINKER"
                   : (ButtonClicked(Buttons[GLIDER])  ?  "GLIDER"
                   : (ButtonClicked(Buttons[SHIP])    ?  "SHIP"
                   : (ButtonClicked(Buttons[GUN])     ?  "GUN"
                   : (ButtonClicked(Buttons[PULSAR])  ?  "PULSAR"
                   : (ButtonClicked(Buttons[RESET])   ?  "RESET"
                   :  /*Nothing was clicked*/            "NONE"
                   )))))))))))
                );

La única razón por la que no usé una cadena if-else fue que hubiera hecho que el código fuera inmenso y más difícil de seguir porque todo lo que necesitaba hacer era imprimir una palabra en la pantalla.

Answer 2

Eso es simplemente un código horrible.

• Está mal formateado. No veo la jerarquía de la expresión.
• Incluso si tuviera un buen formato, la expresión sería demasiado compleja para analizar rápidamente con el ojo humano.
• La intención no está clara. ¿Cuál es el propósito de esas condiciones?

¿Entonces que puedes hacer?

• Usa sentencias condicionales ( if ).
• Extraiga las subexpresiones y almacénelas en variables. Mira this bonito ejemplo del catálogo de refactorización.
• Usa las funciones de ayuda. Si la lógica es compleja, use el return temprano s. A nadie le gustan las hendiduras profundas.
• Lo más importante, dale a todo un nombre significativo. La intención debe ser clara por qué algo tiene que ser calculado.

Y para ser claros: no hay nada de malo en el operador ternario. Si se usa juiciosamente, a menudo produce un código que es más fácil de digerir. Evita anidarlos sin embargo. Ocasionalmente uso un segundo nivel si el código es muy claro, e incluso entonces uso paréntesis para que mi pobre cerebro no tenga que hacer ciclos adicionales para descifrar la precedencia del operador.

Preocúpate por los lectores de tu código.

Answer 3

Este es un código terrible.

Si bien a menudo es conveniente inicializar una variable con una sola expresión (por ejemplo, para que podamos hacer que sea const ), esto no es una excusa para escribir código como este. Puede mover la lógica compleja a una función y llamarla para inicializar la variable.

void
example(const int a, const int b)
{
  const auto mything = make_my_thing(a, b);
}

En C ++ 11 y versiones posteriores, también puede usar un lambda para inicializar una variable.

void
example(const int a, const int b)
{
  const auto mything = [a, b](){
      if (a == b)
        return MyThing {"equal"};
      else if (a < b)
        return MyThing {"less"};
      else if (a > b)
        return MyThing {"greater"};
      else
        throw MyException {"How is this even possible?"};
  }();
}

Answer 4

Otros ya dijeron lo horrible que es el extracto del código, con buenas explicaciones. Solo daré algunas razones más por las que ese código es malo:

1. Si considera que un "if-else" implementa exactamente una característica, entonces está claro cuán complejo es ese código. En su caso, ni siquiera puedo contar el número de ifs.

2. Es obvio que su código rompe el principio de responsabilidad única , que dice:

   ... una clase o módulo debe tener una, y solo una, razón para cambiar.

3. Pruebas unitarias que serían una pesadilla, que es otra bandera roja. Y apuesto a que su colega ni siquiera intentó escribir pruebas unitarias para esa pieza de código.

Answer 5

Qué feo lío. Rompí en si y si no para ver lo que estaba haciendo. No es mucho más legible, pero pensé que lo publicaría de todos modos. Esperemos que alguien más tenga una solución más elegante para ti. Pero para responder a tu pregunta, no uses ternarios tan complicados. Nadie quiere hacer lo que acabo de hacer para averiguar lo que está haciendo.

if ( req.security_violation )
{
    if ( dis_prot_viol_rsp && is_mstr )
    {
        exp_rsp_status = uvc_pkg::MRSP_OKAY;
    }
    else
    {
        exp_rsp_status = uvc_pkg::MRSP_PROTVIOL;
    }
}
else if ( req.slv_req.size() )
{
    if ( ( is_mst_abort_rsp && dis_mst_abort_rsp ||
         ( req.slv_req[0].get_rsp_status() == uvc_pkg::MRSP_PROTVIOL && dis_prot_viol_rsp ) ||
         ( is_mst_abort_rsp && req.is_pci_config_req() && dis_pcicfg_mst_abort_rsp ) )
    {
        exp_rsp_status = uvc_pkg::MRSP_OKAY;
    }
    else
    {
        exp_rsp_status = req.slv_req[0].get_rsp_status();
    }

}
else
{
    exp_rsp_status = uvc_pkg::MRSP_OKAY
} 

Answer 6

Tal vez esto se encuentre en el bucle de mensajes del controlador de un dispositivo y el codificador original, posiblemente hace 10 años, no quería saltos en el código. ¡Espero que haya verificado que su compilador no implementó el operador ternario con saltos!

Al examinar el código, mi primera observación es que una secuencia de operadores ternarios es, como todo código, mejor legible cuando está adecuadamente formateada.

Dicho esto, no estoy seguro de haber analizado correctamente el ejemplo del OP, lo que habla en contra. Incluso una construcción anidada tradicional if-else sería difícil de verificar. Esta expresión viola el paradigma de programación fundamental: Divide y Conquista.

req.security_violation
?   dis_prot_viol_rsp && is_mstr
    ?   uvc_pkg::MRSP_OKAY
    :   uvc_pkg::MRSP_PROTVIOL
:   req.slv_req.size()
    ?       is_mst_abort_rsp && dis_mst_abort_rsp
        ||      req.slv_req[0].get_rsp_status()==uvc_pkg::MRSP_PROTVIOL
            &&  dis_prot_viol_rsp
        ||  is_mst_abort_rsp && req.is_pci_config_req() && dis_pcicfg_mst_abort_rsp
        ?   uvc_pkg::MRSP_OKAY
        : req.slv_req[0].get_rsp_status()
    : uvc_pkg::MRSP_OKAY;

Quería comprobar cómo se ve el código cuando se refacta. Seguro que no es más corto, pero me gusta cómo los nombres de las funciones de habla aclaran la intención (por supuesto que lo adiviné aquí). Esto es, hasta cierto punto, pseudo código porque los nombres de las variables probablemente no son globales, por lo que las funciones deberían tener parámetros, lo que hace que el código sea menos claro nuevamente. Pero quizás el parámetro podría ser un solo puntero a una estructura de estado o solicitud o tal (de la cual se dis_prot_viol_rsp valores como dis_prot_viol_rsp ). El tema de si se debe utilizar o no un ternario cuando se combinan las diferentes condiciones. Lo encuentro a menudo elegante.

bool ismStrProtoViol()
{
    return dis_prot_viol_rsp && is_mstr;
}

bool isIgnorableAbort()
{
    return is_mst_abort_rsp && dis_mst_abort_rsp;
}

bool isIgnorablePciAbort()
{
    return is_mst_abort_rsp && req.is_pci_config_req() && dis_pcicfg_mst_abort_rsp;
}

bool isIgnorableProtoViol()
{
    return req.slv_req[0].get_rsp_status()==uvc_pkg::MRSP_PROTVIOL &&  dis_prot_viol_rsp;
}


eStatus getRspStatus()
{
    eStatus ret;

    if( req.security_violation )
    {
        ret = ismStrProtoViol() ?  uvc_pkg::MRSP_OKAY : uvc_pkg::MRSP_PROTVIOL;
    }
    else if(  req.slv_req.size() )
    {
        ret =       isIgnorableAbort()
                ||  isIgnorableProtoViol()
                ||  isIgnorablePciAbort()
            ? uvc_pkg::MRSP_OKAY
            : req.slv_req[0].get_rsp_status();
    else
    {
        ret = uvc_pkg::MRSP_OKAY;
    }

    return ret;
}

Finalmente, podemos aprovechar el hecho de que uvc_pkg::MRSP_OKAY es un poco el predeterminado y solo se sobrescribe bajo ciertas circunstancias. Esto elimina una rama. Mire cómo después de un poco de cincelado, el razonamiento del código es muy visible: si no es una violación de seguridad, analice más detenidamente y verifique el estado real de la solicitud, menos las solicitudes vacías y los abortos que se pueden ignorar.

eStatus getRspStatus()
{
    eStatus ret = uvc_pkg::MRSP_OKAY;

    if( req.security_violation )
    {
        ret = ismStrProtoViol() ? uvc_pkg::MRSP_OKAY : uvc_pkg::MRSP_PROTVIOL;
    }
    else if(        req.slv_req.size()
                &&  !isIgnorableAbort()
                &&  !isIgnorablePorotoViol()
                &&  !isIgnorablePciAbort()
            )
    {
        ret = req.slv_req[0].get_rsp_status();
    }

    return ret;
}