En C y C ++, ¿qué métodos pueden evitar el uso accidental de la asignación (=) donde se necesita equivalencia (==)?

15

En C y C ++, es muy fácil escribir el siguiente código con un error grave.

char responseChar = getchar();
int confirmExit = 'y' == tolower(responseChar);
if (confirmExit = 1)
{
    exit(0);
}

El error es que la declaración if debería haber sido:

if (confirmExit == 1)

Según lo codificado, saldrá cada vez, porque se confirmExitproduce la asignación de la variable, y luego confirmExitse utiliza como resultado de la expresión.

¿Hay buenas maneras de prevenir este tipo de error?

c++ coding-standards DesarrolladorDon
fuente
39
Si. Activa las advertencias del compilador. Trate las advertencias como errores y no es un problema.
Martin York
posible duplicado de ¿No hace "if (0 == value) ..." más daño que bien?
Martin York
99
En el ejemplo dado, la solución es fácil. Le asigna un valor booleano, por tanto, lo utilizan como un valor lógico: if (confirmExit).
Seguro el
44
El problema es que el error fue cometido por los "diseñadores" del lenguaje C, cuando eligieron usar = para el operador de asignación y == para la comparación de igualdad. ALGOL usó: =, porque específicamente querían usar = para la comparación de igualdad, y PASCAL y Ada siguieron la decisión de ALGOL. (Vale la pena señalar que, cuando el Departamento de Defensa solicitó una entrada basada en C en el horneado de DoD1, que finalmente produjo a Ada, Bell Labs declinó, diciendo que "C no era ahora y nunca sería lo suficientemente robusto para la misión crítica del Departamento de Defensa. software ". Uno desearía que el Departamento de Defensa y los contratistas hayan escuchado a Bell Labs sobre esto.)
John R. Strohm
44
@John, la elección de los símbolos no es el problema, es el hecho de que las asignaciones también son una expresión que devuelve el valor asignado, permitiendo a = bo a == bdentro de un condicional.
Karl Bielefeldt

Respuestas:

60

La mejor técnica es aumentar el nivel de advertencia de su compilador. Luego le advertirá sobre la posible asignación en el condicional if.

Asegúrese de compilar su código con cero advertencias (lo que debería estar haciendo de todos modos). Si desea ser pedante, configure su compilador para tratar las advertencias como errores.

El uso de condicionales Yoda (poner la constante en el lado izquierdo) fue otra técnica que fue popular hace aproximadamente una década. Pero hacen que el código sea más difícil de leer (y, por lo tanto, se mantienen debido a la forma poco natural en que leen (a menos que sea Yoda)) y no brindan un beneficio mayor que el aumento del nivel de advertencia (que también tiene beneficios adicionales de más advertencias).

Las advertencias son realmente errores lógicos en el código y deben corregirse.

Martin York
fuente
2
Definitivamente, vaya con las advertencias, no con los condicionales de Yoda: puede olvidarse de hacer la constante condicional primero tan fácilmente como puede olvidarse de hacer ==.
Michael Kohne el
8
Me sorprendió gratamente que la respuesta más votada para esta pregunta es 'convertir las advertencias del compilador en errores'. Esperaba el if (0 == ret)horror.
James
2
@ James: ... ¡sin mencionar que no funcionará a == b!
Emilio Garavaglia
66
@EmilioGaravaglia: Hay una solución fácil para eso: solo escriba 0==a && 0==b || 1==a && 1==b || 2==a && 2==b || ...(repita para todos los valores posibles). No olvide el ...|| 22==a && 22==b || 23==a && 24==b || 25==a && 25==b ||error obligatorio ... o los programadores de mantenimiento no se divertirán.
user281377
10

Siempre puedes hacer algo radical como probar tu software. Ni siquiera me refiero a las pruebas unitarias automatizadas, solo las pruebas que cada desarrollador experimentado hace por costumbre al ejecutar su nuevo código dos veces, una vez que confirma la salida y otra vez que no. Esa es la razón por la cual la mayoría de los programadores lo consideran un problema.

Karl Bielefeldt
fuente
1
Fácil de hacer cuando el comportamiento de su programa es completamente determinista.
James
3
Este problema en particular puede ser difícil de probar. He visto a personas mordidas rc=MethodThatRarelyFails(); if(rc = SUCCESS){más de una vez, especialmente si el método falla solo en condiciones difíciles de evaluar.
Gort the Robot
3
@StevenBurnap: Para eso están los objetos simulados. Las pruebas adecuadas incluyen pruebas de modos de falla.
Jan Hudec
Esta es la respuesta correcta.
Lightness compite con Monica el
6

Una forma tradicional de evitar el uso incorrecto de las asignaciones dentro de la expresión es colocar la constante a la izquierda y la variable a la derecha.

if (confirmExit = 1)  // Unsafe

if (1=confirmExit)    // Safe and detected at compile time.

El compilador informará un error para la asignación ilegal a una constante similar a la siguiente.

.\confirmExit\main.cpp:15: error: C2106: '=' : left operand must be l-value

La condición if revisada sería:

  if (1==confirmExit)

Como se muestra en los comentarios a continuación, muchos lo consideran un método inapropiado.

DesarrolladorDon
fuente
8
También conocidos como condicionales Yoda: wiert.me/2010/05/25/… y stackoverflow.com/questions/8591182/… y programmers.stackexchange.com/questions/16908/…
Martin York
13
Cabe señalar que hacer las cosas de esta manera te hará bastante impopular.
Riwalk
11
Por favor no recomiende esta práctica.
James
55
Tampoco funciona si necesita comparar dos variables entre sí.
dan04
Algunos idiomas en realidad permiten asignar 1 a un nuevo valor (sí, sé que Q es un c / c ++)
Matsemann
4

Estoy de acuerdo con que todos digan "advertencias del compilador" pero quiero agregar otra técnica: Revisiones de código. Si tiene una política de revisar todo el código que se compromete, preferiblemente antes de que se confirme, es probable que este tipo de cosas se detecte durante la revisión.

MatrixFrog
fuente
Estoy en desacuerdo. Especialmente = en lugar de == puede deslizarse fácilmente revisando el código.
Simon
2

Primero, elevar sus niveles de advertencia nunca está de más.

Si no desea que su condicional pruebe el resultado de una asignación dentro de la declaración if en sí, entonces haber trabajado con muchos programadores de C y C ++ a lo largo de los años, y nunca haber escuchado que comparar la constante primero if(1 == val)era algo malo, usted podría intentar esa construcción.

Si el líder de su proyecto aprueba que haga esto, no se preocupe por lo que piensen los demás. La verdadera prueba es si usted u otra persona pueden darle sentido a su código dentro de meses y años.

Sin embargo, si su intención era probar el resultado de una asignación, el uso de advertencias más altas podría [probablemente haber] capturado la asignación a una constante.

octopusgrabbus
fuente
Levanto una ceja escéptica ante cualquier programador no principiante que vea un Yoda condicional y no lo entienda de inmediato. Es solo un cambio, no es difícil de leer, y ciertamente no es tan malo como dicen algunos comentarios.
underscore_d
@underscore_d En la mayoría de mis empleadores, las tareas dentro de un condicional estaban mal vistas. La idea era que era mejor separar la asignación de la condicional. La razón para ser más claro, incluso a expensas de otra línea de código, fue el factor de ingeniería sostenido. Trabajé en lugares que tenían grandes bases de códigos y mucho mantenimiento atrasado. Entiendo completamente que alguien podría querer asignar un valor, y ramificarse en la condición resultante de la asignación. Veo que se hace con mayor frecuencia en Perl y, si la intención es clara, seguiré el patrón de diseño del autor.
octopusgrabbus el
Estaba pensando en los condicionales de Yoda solos (como su demostración aquí), no con asignación (como en el OP). No me importa lo primero, pero tampoco me gusta mucho lo último. La única forma que uso deliberadamente es if ( auto myPtr = dynamic_cast<some_ptr>(testPtr) ) {que evita mantener un nullptralcance inútil si falla el lanzamiento, lo que presumiblemente es la razón por la que C ++ tiene esta capacidad limitada para asignar dentro de un condicional. Por lo demás, sí, una definición debería tener su propia línea, yo diría: mucho más fácil de ver de un vistazo y menos propensa a diversos descuidos.
underscore_d
@underscore_d Respuesta editada basada en tus comentarios. Buen punto.
octopusgrabbus el
1

Tarde a la fiesta como siempre, pero el análisis de código estático es la clave aquí

La mayoría de los IDE ahora proporcionan SCA más allá de la verificación sintáctica del compilador, y hay otras herramientas disponibles, incluidas las que implementan las directrices MISRA (*) y / o CERT-C.

Declaración: formo parte del grupo de trabajo MISRA C, pero estoy publicando a título personal. También soy independiente de cualquier proveedor de herramientas

Andrés
fuente
-2

Solo use la asignación de la mano izquierda, las advertencias del compilador pueden ayudar, pero debe asegurarse de obtener el nivel correcto; de lo contrario, se verá inundado de advertencias sin sentido o no se le dirán las que desea ver.

Llama invertida
fuente
44
Te sorprenderías de las pocas advertencias sin sentido que generan los compiladores modernos. Es posible que no piense que una advertencia es importante, pero en la mayoría de los casos simplemente está equivocado.
Kristof Provost el
Echa un vistazo al libro "Writing Solid Code" amazon.com/Writing-Solid-Microsoft-Programming-Series/dp/… . Comienza con una gran discusión sobre compiladores y cuánto beneficio podríamos obtener de los mensajes de advertencia e incluso un análisis estático más extenso.
DesarrolladorDon
@ KristofProvost Me las arreglé para que Visual Studio me diera 10 copias de la misma advertencia exacta para la misma línea de código, luego, cuando este problema se 'solucionó', resultó en 10 advertencias idénticas sobre la misma línea de código diciendo que el El método original fue mejor.
Llama invertida el