Comprobación del resultado de un constructor en C #

Estoy trabajando en una base de código con un compañero de trabajo que tiene la costumbre de verificar los resultados de un constructor para un nulo de una manera similar a esta

Person p = new Person();
if (p != null)
{
    p.Name = "John Smith";
}

Mi comprensión del panorama .NET es que un constructor nunca dejará una tarea sin cumplir a menos que se lance una excepción. Entonces, en el caso anterior, la verificación nula es inútil. Se pasigna o se generará una excepción que omitirá el configurador de propiedades.

Le pregunté a mi compañero de trabajo sobre esto de pasada, y recibí una respuesta pasiva en la línea de "por si acaso". No me gusta este tipo de "programación de paronia". Creo que perjudica la legibilidad y aumenta innecesariamente la complejidad ciclomática. Debido a esto, me gustaría hacer una solicitud formal para que esta práctica se detenga. ¿Es esta una solicitud razonable? ¿Me estoy perdiendo de algo?

Estoy de acuerdo con @MartinMaat sobre elegir tus batallas.

Hay muchos casos de "por si acaso" debido a que realmente no comprende el idioma a pesar de que el lenguaje se fija en sus reglas para muchas de estas cosas, sobre paréntesis de una expresión que no lo necesita debido a que no comprende las reglas de precedencia de el idioma. Pero aún así, tal práctica es mayormente inofensiva.

Cuando era más joven, sentí que deberíamos aprender los detalles del idioma y así evitar escribir un código tan superfluo. (Una de mis manías era return (0);con sus parentes innecesarios). Sin embargo, ahora modero esa posición, en particular, porque ahora usamos tantos idiomas diferentes, saltando de cliente a servidor, etc. Así que ahora corté algunos flojo para algunos de esos problemas.

Su punto sobre ciclomático comienza a ir a un argumento lógicamente razonado. Echemos un vistazo a la cobertura de código y especialmente a los niveles más altos de cobertura :

2. Cada decisión toma todos los resultados posibles

Como no podemos obligar a la nueva operación a devolver NULL, no hay forma de alcanzar niveles más altos de cobertura de código para esta operación condicional.   ¡Por supuesto, esto puede o no ser importante para su organización!

Sin embargo, debido a este problema de cobertura del código, lo priorizaría más que al paréntesis excesivo.

Por otro lado, el código generado subyacente probablemente no sufrirá un poco por esto ya que las generaciones de código, JIT y optimizadores entienden que un newvalor ed nunca será nulo. Por lo tanto, el costo real solo viene en términos de legibilidad y capacidades de cobertura del código fuente.

Le preguntaría cómo es la "parte-otra" de tal declaración if.

Si no hay otra parte, diría que simplemente caerse del final de la rutina o caer a otro código (es decir, no elsepara esto if) es potencialmente peligroso, ya que ahora "por si acaso" sugiere lógicamente que las personas que llaman y / o código adicional en la línea maneja NULL también.

Si se lee:

p = new Object ();
if ( p != null ) {
    p.field = value;
}
else {
    throw new NullReferenceException ();
}

entonces esto es realmente excesivo, ya que el lenguaje hace todo eso por nosotros.

Podría sugerir invertir el sentido de lo condicional, tal vez su colega se sienta más cómodo con esto:

p = new Object ();
if ( p == null ) {
    throw new NullReferenceException ();
}
else {
    p.field = value;
}

Ahora puede abogar por la eliminación del envoltorio else, ya que es claramente innecesario:

p = new Object ();
if ( p == null ) {
    throw new NullReferenceException ();
}
p.field = value;

Con esto, el "por si acaso" es ahora lo que es condicional, mientras que el código siguiente no lo es. Este enfoque refuerza aún más que cuando la asignación falla, se arroja la respuesta adecuada, en lugar de continuar ejecutando código en este método y / o en esta cadena de llamadas (sin ningún otro manejo adecuado del error de asignación).

Entonces, en resumen, hay dos argumentos lógicamente razonados para hacer aquí en contra de esta práctica:

1. No se pueden alcanzar niveles de cobertura de código más altos ya que no podemos forzar la falta de memoria (o cualquier falla del constructor) para devolver nulo.
2. El "por si acaso" (como se muestra arriba en la pregunta) está incompleto y, como tal, es defectuoso debido a la inconsistencia en las expectativas de cuán nulo sería manejado por otro código más allá del pasado p.field = value;.

Básicamente, parece que tal vez su colega está cerca de usar excepciones, a pesar de que aquí en C # no hay otra opción para tales cosas. ( Si queremos un código bien probado, no podemos codificar tanto un modelo de excepción para manejar un modelo nulo como uno que no sea de excepción usando valores de retorno nulo, uno al lado del otro). Quizás si razona con su colega a través de estos temas, ¡Verán algo de luz!

Quizás pueda esquivar el problema y simplemente hacerle saber que hay una nueva sintaxis de C # que es más fácil (¡y la verificación nula lo hace por usted!)

En vez de

Person p = new Person();
if (p != null)
{
    p.Name = "John Smith";
}

Escribir

var p = new Person
{
    Name = "John Smith"
};

Es una solicitud razonable, pero parece que ya se ha convertido en una lucha entre usted y él. Es posible que ya se lo haya señalado, él es reacio a cambiarlo y ahora planea escalar el asunto. Entonces puedes "ganar".

Puede ser más sencillo enviarle el enlace que Robert Harvey publicó y decir "Te envié un enlace sobre la construcción de objetos de C # que puedes encontrar interesante" y dejarlo allí. No es exactamente perjudicial lo que hace (simplemente no tiene sentido) y es fácil de limpiar después. Estoy diciendo: elige tus batallas. He estado en esta situación más de una vez y en retrospectiva, a menudo simplemente no valía la pena la lucha. Muy fácilmente se odian mutuamente por casi nada. Sí, tiene razón, pero aunque solo sean usted y él, es posible que desee preservar un terreno común.

Como su pregunta técnica ya ha sido respondida (la verificación nula no tiene sentido), me gustaría centrarme en un aspecto diferente de su pregunta: cómo lidiar con tal situación en general: un compañero de trabajo que no conoce algunos hechos básicos acerca de las herramientas que todos usan, y por lo tanto las usa de una manera menos que óptima.

En algunos casos, puede no afectarlo mucho. Si hacen su trabajo y no interfieren con el tuyo, puedes ignorar el problema.

En otros casos (por ejemplo, el suyo), que no interfieren con su trabajo, al menos en cierta medida. ¿Qué hacer al respecto?

Creo que depende de muchos detalles. Por ejemplo, cuánto tiempo llevan usted y su compañero de trabajo con la empresa, cómo la empresa generalmente se ocupa de tales problemas, cuánto le afecta / molesta el problema, qué tan bien se lleva con el compañero de trabajo, cuán importante es un armonioso relación con su compañero de trabajo con usted, en qué medida podría plantear o intensificar el problema afectar esta relación, etc.

Mi opinión personal podría ser esta: creo que los ingenieros deberían conocer sus herramientas, y los ingenieros de software deberían conocer sus lenguajes de programación. Por supuesto, nadie conoce todos los detalles, pero que los constructores nunca regresen nulles una idea muy básica: todos deberían saberlo. Cuando necesito trabajar en algún código que contenga tales comprobaciones nulas sin sentido, simplemente las eliminaré. Cuando un colega me pregunta por qué los eliminé, le explico por qué no tienen sentido. (Si es necesario, también explicaría por qué el código que es completamente innecesario debería eliminarse). A la larga, esto conduciría a un código que esté libre de estas comprobaciones nulas sin sentido.

Esto puede sonar algo arrogante, y puede no ser el enfoque correcto para todos. Me han dicho que soy paciente y bueno para explicar las cosas. Tal vez es por eso que tiendo a salirse con la suya sin parecer un imbécil. :-)

Usted menciona "una solicitud formal para que se detenga esta práctica". Nunca he trabajado en una empresa que tuviera reglas formales tan detalladas, pero de todos modos, aquí hay algunas ideas. Nuevamente, creo que su mejor curso de acción depende de los detalles. Si presentar una solicitud de este tipo haría que su compañero de trabajo se viera mal a los ojos de colegas o gerentes, probablemente no lo haría. Por otro lado, si tales solicitudes generalmente se consideran una mejora bienvenida y otros no culparán a su compañero de trabajo por no seguir esta regla antes, continúe. Nadie será lastimado, todos estarán mejor.

Pero si no pudiera encontrar una buena manera de lidiar con el problema, podría recurrir al sarcasmo y agregar código como este en todas partes:

int c = a + b;
if (c == a + b)
{
    // next steps
}
else 
{
    throw new ArithmeticException();
}

Es tan inútil como una verificación nula después de una llamada de constructor, pero aún más obvio. Y, por supuesto, realmente no haría esto. A menos que esté tan molesto con mis compañeros de trabajo que estoy planeando dejarlo de todos modos. ;-)