En un nuevo trabajo, me han marcado las revisiones de código para códigos como este:
PowerManager::PowerManager(IMsgSender* msgSender)
: msgSender_(msgSender) { }
void PowerManager::SignalShutdown()
{
msgSender_->sendMsg("shutdown()");
}
Me han dicho que el último método debería leer:
void PowerManager::SignalShutdown()
{
if (msgSender_) {
msgSender_->sendMsg("shutdown()");
}
}
es decir, debo colocar un NULL
guard alrededor de la variable msgSender_
, aunque sea un miembro de datos privados. Es difícil para mí abstenerme de usar explosivos para describir cómo me siento con respecto a esta pieza de "sabiduría". Cuando pido una explicación, recibo una letanía de historias de horror sobre cómo un programador junior, durante algún año, se confundió sobre cómo se suponía que funcionaba una clase y eliminó accidentalmente un miembro que no debería haber (y lo establece en NULL
después, aparentemente), y las cosas explotaron en el campo justo después del lanzamiento de un producto, y "hemos aprendido de la manera más difícil, confíe en nosotros" que es mejor simplemente NULL
revisar todo .
Para mí, esto se siente como programación de culto a la carga , simple y llanamente. Unos cuantos colegas bienintencionados están tratando seriamente de ayudarme a 'conseguirlo' y ver cómo esto me ayudará a escribir un código más robusto, pero ... No puedo evitar sentir que son los que no lo entienden. .
¿Es razonable que un estándar de codificación requiera que se revise primero el puntero de cada uno en una función en busca de NULL
primero, incluso los miembros de datos privados? (Nota: para dar un poco de contexto, creamos un dispositivo de electrónica de consumo, no un sistema de control de tráfico aéreo o algún otro producto de 'fallos iguales a personas mueren').
EDIT : en el ejemplo anterior, el colaborador msgSender_
no es opcional. Si alguna vez es NULL
, indica un error. La única razón por la que se pasa al constructor es para que PowerManager
se pueda probar con una subclase simulada IMsgSender
.
RESUMEN : Hubo algunas respuestas realmente buenas a esta pregunta, gracias a todos. Acepté el de @aaronps principalmente por su brevedad. Parece haber un acuerdo general bastante amplio de que:
- Mandar a
NULL
guardias para todos los punteros a los que no se hace referencia a referencias individuales es excesivo, pero - Puede hacer un lado de todo el debate utilizando una referencia en su lugar (si es posible) o un puntero
const
, y -
Las declaraciones
assert
son una alternativa más inteligente aNULL
guardias para verificar que se cumplan las condiciones previas de una función.