Comprobación de parámetros nulos en C #

85

En C #, ¿hay alguna buena razón (aparte de un mejor mensaje de error) para agregar verificaciones nulas de parámetros a cada función donde nulo no es un valor válido? Obviamente, el código que usa s lanzará una excepción de todos modos. Y tales comprobaciones hacen que el código sea más lento y difícil de mantener.

void f(SomeType s)
{
  if (s == null)
  {
    throw new ArgumentNullException("s cannot be null.");
  }

  // Use s
}
Kaalus
fuente
12
Dudo seriamente que una simple verificación nula ralentice su código en una cantidad significativa (o incluso mensurable).
Heinzi
Bueno, depende de lo que haga "Use s". Si todo lo que hace es "devolver en algún campo", la comprobación adicional probablemente ralentizará el método en un orden de magnitud.
Kaalus
10
@kaalus: ¿Probablemente? Probablemente no lo corte en mi libro. ¿Dónde están sus datos de prueba? ¿Y qué tan probable es que tal cambio sea el cuello de botella de su aplicación en primer lugar?
Jon Skeet
@Jon: tienes razón, no tengo datos concretos aquí. Si desarrolla para Windows Phone o Xbox, donde la alineación JIT se verá afectada por este "si" adicional, y donde la ramificación es muy costosa, puede volverse bastante paranoico.
Kaalus
3
@kaalus: Por lo tanto, ajuste el estilo de su código en esos casos muy específicos después de probar que hace una diferencia significativa o use Debug.Assert (nuevamente, solo en esas situaciones, vea la respuesta de Eric) para que las verificaciones solo estén activas en ciertas compilaciones.
Jon Skeet

Respuestas:

163

Sí, hay buenas razones:

  • Identifica exactamente lo que es nulo, que puede no ser obvio a partir de un NullReferenceException
  • Hace que el código falle en una entrada no válida incluso si alguna otra condición significa que el valor no está desreferenciado
  • Hace que la excepción ocurra antes de que el método pueda tener cualquier otro efecto secundario que pueda alcanzar antes de la primera desreferencia.
  • Significa que puede estar seguro de que si pasa el parámetro a otra cosa, no está violando su contrato.
  • Documenta los requisitos de su método (el uso de contratos de código es aún mejor para eso, por supuesto)

Ahora en cuanto a sus objeciones:

  • Es más lento : ¿Ha encontrado que esto es realmente el cuello de botella en su código, o lo está adivinando? Las comprobaciones de nulidad son muy rápidas y, en la gran mayoría de los casos, no serán el cuello de botella.
  • Hace que el código sea más difícil de mantener : creo que lo contrario. Creo que es más fácil usar código donde queda muy claro si un parámetro puede ser nulo o no, y donde está seguro de que esa condición se aplica.

Y para tu afirmación:

Obviamente, el código que usa s lanzará una excepción de todos modos.

De Verdad? Considerar:

void f(SomeType s)
{
  // Use s
  Console.WriteLine("I've got a message of {0}", s);
}

Eso usa s, pero no lanza una excepción. Si no es válido para sser nulo, y eso indica que algo anda mal, una excepción es el comportamiento más apropiado aquí.

Ahora, dónde colocas esas comprobaciones de validación de argumentos es un asunto diferente. Puede decidir confiar en todo el código dentro de su propia clase, así que no se preocupe por los métodos privados. Puede decidir confiar en el resto de su ensamblaje, así que no se preocupe por los métodos internos. Es casi seguro que debería validar los argumentos de los métodos públicos.

Una nota al margen: la sobrecarga del constructor de un solo parámetro de ArgumentNullExceptiondebería ser solo el nombre del parámetro, por lo que su prueba debería ser:

if (s == null)
{
  throw new ArgumentNullException("s");
}

Alternativamente, puede crear un método de extensión, permitiendo algo más terso:

s.ThrowIfNull("s");

En mi versión del método de extensión (genérico), hago que devuelva el valor original si no es nulo, lo que le permite escribir cosas como:

this.name = name.ThrowIfNull("name");

También puede tener una sobrecarga que no toma el nombre del parámetro, si no le preocupa demasiado.

Jon Skeet
fuente
8
+1 para el método de extensión. Hice exactamente lo mismo, incluida otra validación como ThrowIfEmptyelICollection
Davy8
5
Por qué creo que es más difícil de mantener: al igual que con todos los mecanismos manuales que requieren la intervención del programador cada vez, es propenso a errores y omisiones y satura el código. La seguridad escamosa es peor que la falta de seguridad.
kaalus
8
@kaalus: ¿Aplicas la misma actitud a las pruebas? "Mis pruebas no detectarán todos los errores posibles, por lo tanto, no escribiré ninguno"? Cuando los mecanismos de seguridad hacen el trabajo, que va a hacer que sea más fácil encontrar el problema y reducir el impacto en otros lugares (por controlar el problema anterior). Si eso sucede 9 de cada 10 veces, aún es mejor que 0 de cada 10 veces ...
Jon Skeet
2
@DoctorOreo: No lo uso Debug.Assert. Es incluso más importante detectar errores en la producción (antes de que estropeen los datos reales) que en el desarrollo.
Jon Skeet
2
Ahora con C # 6.0 incluso podemos usar throw new ArgumentNullException(nameof(s))
hrzafer
52

Estoy de acuerdo con Jon, pero agregaría una cosa a eso.

Mi actitud sobre cuándo agregar comprobaciones nulas explícitas se basa en estas premisas:

  • Debe haber una forma de que sus pruebas unitarias ejerciten cada declaración en un programa.
  • throwlas declaraciones son declaraciones .
  • La consecuencia de una ifes una declaración .
  • Por lo tanto, debe haber una manera de ejercer el throwenif (x == null) throw whatever;

Si no hay forma posible de ejecutar esa declaración, no se puede probar y se debe reemplazar por Debug.Assert(x != null);.

Si hay una forma posible de ejecutar esa declaración, escriba la declaración y luego escriba una prueba unitaria que la ejercite.

Es particularmente importante que los métodos públicos de tipo público verifiquen sus argumentos de esta manera; no tienes idea de la locura que van a hacer tus usuarios. Dales el "oye, idiota, ¡lo estás haciendo mal!" excepción tan pronto como sea posible.

Por el contrario, es mucho más probable que los métodos privados de tipos privados se encuentren en una situación en la que controle los argumentos y pueda tener una fuerte garantía de que el argumento nunca sea nulo; use una afirmación para documentar ese invariante.

Eric Lippert
fuente
22

He estado usando esto durante un año:

_ = s ?? throw new ArgumentNullException(nameof(s));

Es un delineador único y el descarte ( _) significa que no hay asignación innecesaria.

galdin
fuente
8

Sin una ifverificación explícita , puede ser muy difícil averiguar qué fue nullsi no posee el código.

Si obtiene una información NullReferenceExceptionde lo más profundo de una biblioteca sin código fuente, es probable que tenga muchos problemas para averiguar qué hizo mal.

Estas ifcomprobaciones no harán que su código sea notablemente más lento.


Tenga en cuenta que el parámetro del ArgumentNullExceptionconstructor es un nombre de parámetro, no un mensaje.
Tu código debe ser

if (s == null) throw new ArgumentNullException("s");

Escribí un fragmento de código para facilitar esto:

<?xml version="1.0" encoding="utf-8" ?>
<CodeSnippets  xmlns="http://schemas.microsoft.com/VisualStudio/2005/CodeSnippet">
    <CodeSnippet Format="1.0.0">
        <Header>
            <Title>Check for null arguments</Title>
            <Shortcut>tna</Shortcut>
            <Description>Code snippet for throw new ArgumentNullException</Description>
            <Author>SLaks</Author>
            <SnippetTypes>
                <SnippetType>Expansion</SnippetType>
                <SnippetType>SurroundsWith</SnippetType>
            </SnippetTypes>
        </Header>
        <Snippet>
            <Declarations>
                <Literal>
                    <ID>Parameter</ID>
                    <ToolTip>Paremeter to check for null</ToolTip>
                    <Default>value</Default>
                </Literal>
            </Declarations>
            <Code Language="csharp"><![CDATA[if ($Parameter$ == null) throw new ArgumentNullException("$Parameter$");
        $end$]]>
            </Code>
        </Snippet>
    </CodeSnippet>
</CodeSnippets>
SLaks
fuente
5

Es posible que desee echar un vistazo a los contratos de código si necesita una forma más agradable de asegurarse de no obtener ningún objeto nulo como parámetro.

AD.Net
fuente
2

El principal beneficio es que está siendo explícito con los requisitos de su método desde el principio. Esto deja en claro a otros desarrolladores que trabajan en el código que es realmente un error que una persona que llama envíe un valor nulo a su método.

La verificación también detendrá la ejecución del método antes de que se ejecute cualquier otro código. Eso significa que no tendrá que preocuparse por las modificaciones que se realizan con el método que quedan sin terminar.

Justin Niessner
fuente
2

Ahorra algo de depuración, cuando golpea esa excepción.

La excepción ArgumentNullException indica explícitamente que era "s" la que era nula.

Si no tiene esa verificación y deja que el código explote, obtendrá una NullReferenceException de alguna línea no identificada en ese método. ¡En una versión de lanzamiento no obtienes números de línea!

Hans Ke st ing
fuente
0

Codigo original:

void f(SomeType s)
{
  if (s == null)
  {
    throw new ArgumentNullException("s cannot be null.");
  }

  // Use s
}

Reescribirlo como:

void f(SomeType s)
{
  if (s == null) throw new ArgumentNullException(nameof(s));
}

[Editar] La razón para reescribir el uso nameofes porque permite una refactorización más fácil. Si el nombre de su variable salguna vez cambia, los mensajes de depuración también se actualizarán, mientras que si solo codifica el nombre de la variable, eventualmente quedará desactualizado cuando se realicen actualizaciones con el tiempo. Es una buena práctica utilizada en la industria.

Asher Garland
fuente
Si está sugiriendo un cambio, explique por qué. También asegúrese de responder la pregunta que se le hace.
CodeCaster
-1
int i = Age ?? 0;

Entonces, para tu ejemplo:

if (age == null || age == 0)

O:

if (age.GetValueOrDefault(0) == 0)

O:

if ((age ?? 0) == 0)

O ternario:

int i = age.HasValue ? age.Value : 0;
Muhammad Farooq Khalid
fuente