Si las funciones tienen que hacer verificaciones nulas antes de realizar el comportamiento deseado, ¿este diseño es malo?

El problema con su ejemplo básico no es la comprobación nula, es el fallo silencioso .

Los errores de referencia / puntero nulos, en la mayoría de los casos, son errores del programador. Los errores del programador a menudo se tratan mejor fallando de inmediato y en voz alta. Tiene tres formas generales de solucionar el problema en esta situación:

1. No se moleste en verificar, y simplemente deje que el tiempo de ejecución lance la excepción de puntero nulo.
2. Marque y lance una excepción con un mensaje mejor que el mensaje NPE básico.

Una tercera solución es más trabajo pero es mucho más robusta:

3. Estructure su clase de modo que sea prácticamente imposible que _someEntity esté en un estado no válido. Una forma de hacerlo es deshacerse de AssignEntity y exigirlo como parámetro para la creación de instancias. Otras técnicas de inyección de dependencia también pueden ser útiles.

En algunas situaciones, tiene sentido verificar la validez de todos los argumentos de la función antes de cualquier trabajo que esté realizando, y en otras tiene sentido decirle a la persona que llama que es responsable de garantizar que sus entradas sean válidas y no de verificación. El extremo del espectro en el que se encuentre dependerá de su dominio de problema. La tercera opción tiene un beneficio significativo, ya que puede, hasta cierto punto, hacer que el compilador haga que el llamador haga todo correctamente.

Si la tercera opción no es una opción, en mi opinión, realmente no importa mientras no falle silenciosamente. Si no se comprueba si el nulo hace que el programa explote instantáneamente, está bien, pero si, en cambio, corrompe los datos para causar problemas en el futuro, es mejor verificarlos y tratarlos en ese mismo momento.

Dado que _someEntity puede modificarse en cualquier etapa, entonces tiene sentido probarlo cada vez que se llame a SetName . Después de todo, podría haber cambiado desde la última vez que se llamó a ese método. Pero tenga en cuenta que el código en SetName no es seguro para subprocesos, por lo que puede realizar esa verificación en un subproceso, tener _someEntity establecido en null por otro y luego el código emitirá un error de NullReferenceException de todos modos.

Por lo tanto, un enfoque de este problema es ponerse aún más a la defensiva y realizar una de las siguientes acciones:

1. haga una copia local de _someEntity y haga referencia a esa copia a través del método,
2. use un bloqueo alrededor de _someEntity para asegurarse de que no cambie mientras se ejecuta el método,
3. use un try/catch y realice la misma acción en cualquier NullReferenceException que ocurra dentro del método como lo haría en la verificación nula inicial (en este caso, una acción nop ).

Pero lo que realmente debería hacer es detenerse y hacerse la pregunta: ¿realmente necesita permitir que se sobrescriba _someEntity ? Por qué no configurarlo una vez, a través del constructor. De esa manera, solo necesitas hacer esa comprobación nula una vez:

private readonly Entity _someEntity;

public Constructor(Entity someEntity)
{
    if (someEntity == null) throw new ArgumentNullException(nameof(someEntity));
    _someEntity = someEntity;
}

public void SetName(string name)
{
    _someEntity.Name = name;
    label.SetText(_someEntity.Name);
}

Si es posible, este es el enfoque que recomendaría tomar en tales situaciones. Si no puede tener en cuenta la seguridad de los subprocesos al elegir cómo manejar los posibles nulos.

Como comentario adicional, creo que su código es extraño y podría mejorarse. Todos:

private Entity _someEntity;
public Entity SomeEntity => _someEntity;

public void AssignEntity(Entity entity){
    _someEntity = entity;
}

se puede reemplazar por:

public Entity SomeEntity { get; set; }

Que tiene la misma funcionalidad, guardar para poder establecer el campo a través del establecedor de propiedades, en lugar de un método con un nombre diferente.

  

¿Pero el método no debería tener esta comprobación?

Esta es la elección de tu .

Al crear un método public , le está ofreciendo a public la oportunidad de llamarlo. Esto siempre viene con un contrato implícito sobre cómo para llamar a ese método y qué esperar al hacerlo. Este contrato puede (o no) incluir " sí, uhhm, si pasa null como valor de parámetro, explotará directamente en su cara. ". Otras partes de ese contrato pueden ser " oh, BTW: cada vez que dos subprocesos ejecutan este método al mismo tiempo, un gatito muere ".

El tipo de contrato que quieras diseñar depende de ti. Las cosas importantes son

• cree una API que sea útil y coherente y

• para documentarlo bien, especialmente para esos casos de borde.

¿Cuáles son los casos probables de cómo se llama su método?

Las otras respuestas son buenas; Me gustaría ampliarlos haciendo algunos comentarios sobre los modificadores de accesibilidad. Primero, qué hacer si null nunca es válido:

Los métodos

• públicos y protegidos son aquellos en los que no controlas a la persona que llama. Deben tirar cuando se pasa nulo. De esa manera usted entrena a sus interlocutores para que nunca le pasen un nulo, porque les gustaría que su programa no se bloquee.

• Los métodos internos y privados son aquellos en los que do controla a la persona que llama. Deben afirmar cuando se pasa nulo; entonces probablemente se estrellarán más tarde, pero al menos obtienes la afirmación primero, y la oportunidad de entrar en el depurador. Nuevamente, desea capacitar a sus interlocutores para que lo llamen correctamente, haciendo que duela cuando no lo hacen.

Ahora, ¿qué hace si podría ser válido para que se pase un nulo? Yo evitaría esta situación con el patrón de objeto nulo . Es decir: crear una instancia especial del tipo y requerir que se use cada vez que se necesite un objeto no válido . Ahora estás de vuelta en el placentero mundo de lanzar / afirmar en cada uso de null.

Por ejemplo, no quieres estar en esta situación:

class Person { ... }
...
class ExpenseReport { 
  public void SubmitReport(Person approver) { 
    if (approver == null) ... do something ...

y en el sitio de la llamada:

// I guess this works but I don't know what it means
expenses.SubmitReport(null); 

Porque (1) está entrenando a sus interlocutores para que el valor nulo sea un buen valor y (2) no está claro cuál es la semántica de ese valor. Más bien, haz esto:

class ExpenseReport { 
  public static readonly Person ApproverNotRequired = new Person();
  public static readonly Person ApproverUnknown = new Person();
  ...
  public void SubmitReport(Person approver) { 
    if (approver == null) throw ...
    if (approver == ApproverNotRequired) ... do something ...
    if (approver == ApproverUnknown) ... do something ...

Y ahora el sitio de llamadas tiene este aspecto:

expenses.SubmitReport(ExpenseReport.ApproverNotRequired);

y ahora el lector del código sabe lo que significa.

Las otras respuestas señalan que su código se puede limpiar para no necesitar una comprobación nula donde lo tiene, sin embargo, para obtener una respuesta general sobre lo que puede ser una comprobación nula útil, considere el siguiente código de ejemplo: >

public static class Foo {
    public static void Frob(Bar a, Bar b) {
        if (a == null) throw new ArgumentNullException(nameof(a));
        if (b == null) throw new ArgumentNullException(nameof(b));

        a.Something = b.Something;
    }
}

Esto le da una ventaja para el mantenimiento. Si alguna vez ocurre un defecto en su código donde algo pasa un objeto nulo al método, obtendrá información útil del método en cuanto a qué parámetro fue nulo. Sin los controles, obtendrá una NullReferenceException en la línea:

a.Something = b.Something;

Y (dado que la propiedad Something es un tipo de valor) a o b podría ser nulo y no tendrá forma de saber cuál es el rastro de la pila. Con las comprobaciones, sabrá exactamente qué objeto fue nulo, lo que puede ser enormemente útil al intentar eliminar un defecto.

Observaría que el patrón en su código original:

if (x == null) return;

Puede tener sus usos en ciertas circunstancias, también puede (posiblemente más a menudo) darle una muy buena manera de enmascarar los problemas subyacentes en su base de código.

No, en absoluto, pero depende.

Solo hace una comprobación de referencia nula si desea reaccionar.

Si realiza una comprobación de referencia nula y lanza una excepción comprobada , obliga al usuario del método a reaccionar y le permite recuperarse. El programa sigue funcionando como se esperaba.

Si no hace una verificación de referencia nula (o lanza una excepción no verificada), podría lanzar un NullReferenceException sin marcar, que generalmente no es manejado por el usuario del método e incluso podría cerrar la aplicación. Esto significa que la función es obviamente mal entendida y, por lo tanto, la aplicación es defectuosa.

Algo así como una extensión de la respuesta de @ null, pero en mi experiencia, no hago ninguna comprobación.

Una buena programación defensiva es la actitud de codificar la rutina actual de forma aislada, en el supuesto de que el resto del programa intente bloquearla. Cuando estás escribiendo un código de misión crítica donde el fracaso no es una opción, esta es prácticamente la única forma de hacerlo.

Ahora, cómo maneja realmente un valor null , eso depende mucho de su caso de uso.

Si null es un valor aceptable, p. ej. cualquiera de los parámetros segundo a quinto de la rutina MSVC _splitpath (), y luego tratar con ello es el comportamiento correcto.

Si null no debería suceder alguna vez al llamar a la rutina en cuestión, es decir, en el caso del OP, el contrato API requiere que se haya llamado a AssignEntity() con un Entity válido y luego se lance una excepción, o no garantice lo contrario. hacer mucho ruido en el proceso.

Nunca se preocupe por el costo de un chequeo nulo, son muy baratos si ocurren, y con los compiladores modernos, el análisis de código estático puede y los eliminará por completo si el compilador determina que no va a suceder.