¿Es una mala práctica reutilizar los parámetros del método?

12

Hay momentos en que tendré que modificar un valor pasado a un método desde el propio método. Un ejemplo sería desinfectar una cadena como este método aquí:

void SanitizeName(string Name)
{
    Name = Name.ToUpper();

    //now do something here with name
}

Esto es puramente inofensivo ya que el Nameargumento no se pasa por referencia. Sin embargo, si, por alguna razón, un desarrollador en el futuro decide que todos los valores pasen por ref, cualquier desinfección de la cadena afectará el valor fuera del método, lo que podría tener resultados perjudiciales.

Por lo tanto, en lugar de reasignar el argumento en sí, siempre creo una copia local como esta:

void SanitizeName(string Name)
{
    var SanitizedName = Name.ToUpper();

    //now do something here with name
}

Esto garantiza que el cambio en la forma en que se pasa el valor nunca afectará los acontecimientos fuera del método, pero me pregunto si estoy siendo demasiado paranoico al respecto.

oscilantecretina
fuente
1
Sí, es una mala práctica; Y no, no es inofensivo. La línea Name = Name.ToUpper();hace que el código sea más difícil de seguir en la cabeza como el valor de los Namecambios. Su segundo ejemplo no solo es más a prueba de futuro, es más fácil razonar lo que está haciendo.
David Arno
2
¿Pero que pasa if (param == NULL) param = default_value;?
aragaer
Creo que no es tan fácil como sí / no.
MrSmith42
3
Si "un desarrollador en el futuro decide que todos los valores pasen por ref" , probablemente tendría un argumento con ese dev, la reutilización de parámetros involucrada o no ;-) Honestamente, cuando uno decide pasar un valor by refque no se pasó de antemano y, por lo tanto, al convertir el acceso local a un acceso no local por alguna razón, siempre tiene que verificar las consecuencias cuidadosamente.
Doc Brown
2
Principalmente trabajo en un paradigma donde nunca reasignamos ninguna variable. Nunca. Es divertido imaginar a alguien luchando por si deberían reasignar una pequeña minoría de variables y luego no preocuparse por enloquecer con el resto.
Karl Bielefeldt el

Respuestas:

10

Creo que depende de tus convenciones de codificación en tu proyecto.

Personalmente, dejo que eclipse agregue automáticamente la finalpalabra clave a cada variable y parámetro. De esta manera, verá a primera vista si un parámetro se reutiliza.

En el proyecto en mi trabajo no recomendamos reutilizar los parámetros, pero si solo desea llamar, por ejemplo, .trim()o establecer un valor predeterminado en un nullcaso, reutilizamos el parámetro la mayoría de las veces, porque la introducción de una nueva variable es en tales casos menos legible que la reutilización del parámetro.

Realmente no debería reutilizar un parámetro para almacenar un contenido muy diferente porque el nombre ya no se referiría a su contenido. Pero esto es cierto para cada reasignación de una variable y no se limita a los parámetros.

Así que reúnanse con su equipo y formulen convenciones de codificación que cubran este asunto.

MrSmith42
fuente
0

Si utiliza un analizador de código estático por motivos de seguridad, puede confundirse y pensar que no ha validado o desinfectado la variable del parámetro de entrada antes de su uso. Si lo usa Nameen una consulta SQL, por ejemplo, podría reclamar una vulnerabilidad de inyección SQL, lo que le costaría tiempo explicarlo. Eso es malo. Por otro lado, usar una variable con un nombre distintivo para la entrada desinfectada, sin desinfectar realmente la entrada, es una forma rápida de silenciar los analizadores de código ingenuos (hallazgo de vulnerabilidad falsamente negativa).

Greg
fuente
En la mayoría de los casos, también podría usar algún tipo de anotación o comentario especial para indicar al analizador de código que esto es intencional y no un riesgo inadvertido.
MrSmith42
0

La respuesta a esto depende 100% de quién va a leer su código. ¿Qué estilos encuentran más útiles?

He encontrado que el caso más general es que uno evita reasignar valores a argumentos de función porque muchos desarrolladores tienen modelos mentales de cómo funciona la llamada a funciones, lo que supone que nunca se hace esto. Este problema puede ser exacerbado por los depuradores que imprimen los valores de los argumentos con los que llama a cada función. Esta información no es técnicamente correcta si edita los argumentos, y eso puede generar algunas frustraciones extrañas.

Dicho esto, los modelos mentales cambian. En su entorno de desarrollo particular, puede ser deseable tener name"la mejor representación del nombre en este momento". Puede ser deseable vincular más explícitamente las variables en las que está trabajando con sus argumentos, aunque haya realizado algunas modificaciones en sus valores a lo largo del camino. Incluso puede haber ventajas sustanciales en tiempo de ejecución para reutilizar algunas variables particulares en lugar de crear objetos más voluminosos. Después de todo, cuando trabajas con cadenas de 4GB, ¡es bueno minimizar la cantidad de copias adicionales que tienes que hacer!

Cort Ammon
fuente
-1

Tengo un problema completamente diferente con su ejemplo de código:

El nombre de tu método es SanitizeName. En este caso, espero que desinfecte un nombre ; porque eso es lo que le dices al lector de tu función.

Lo único que debe hacer es desinfectar un nombre de pila . Sin leer su código, esperaría lo siguiente:

string SanitizeName(string Name)
{
    //somehow sanitize the name
    return Result;
}

Pero usted implica que su método hace un poco más que solo desinfectar. Ese es un olor a código y debe evitarse.

Su pregunta no es sobre: ¿Es una mala práctica reutilizar los parámetros del método? Es más: ¿los efectos secundarios y el comportamiento no ejecutado son malas prácticas?

La respuesta a eso es claramente: ¡sí!

Esto es puramente inofensivo ya que el argumento Nombre no se pasa por referencia. Sin embargo, si, por alguna razón, un desarrollador en el futuro decide que todos los valores pasen por ref, cualquier desinfección de la cadena afectará el valor fuera del método, lo que podría tener resultados perjudiciales.

Engañas a tu lector al no devolver nada. El nombre de su método indica claramente que está haciendo algo Name. Entonces, ¿a dónde debe ir el resultado? Mediante las voidlecturas de firma de funciones que uso, Namepero no hago ningún daño a la identificación, ni le informo sobre el resultado (explícitamente). Quizás haya una excepción, quizás no; Pero Nameno se altera . Este es el opuesto semántico al nombre de su método.

El problema es menos la reutilización que los efectos secundarios . Para evitar efectos secundarios , no reutilice una variable. Si no tiene efectos secundarios, no hay problema.

Thomas Junk
fuente
44
-1 Esto no responde a la pregunta original. El código proporcionado es solo un código de muestra para mostrar la pregunta en cuestión. Responda la pregunta, en lugar de "atacar" / revisar el código de muestra proporcionado.
Niklas H
1
@NiklasH Esto responde a la pregunta perfectamente: si manipulas el parámetro dentro de la función y, como mencionó el TO, trabajas accidentalmente con una referencia en lugar de una copia, esto causa efectos secundarios (como dije) y esto es malo. Para evitar eso, indique que va a alterar la variable eligiendo cuidadosamente el nombre del método (piense en Ruby '!') Y / o eligiendo un tipo de retorno. Si no modifica la variable, trabaje solo con copias.
Thomas Junk