¿Modificar un parámetro entrante es un antipatrón? [cerrado]

60

Estoy programando en Java, y siempre hago convertidores de esta manera:

public OtherObject MyObject2OtherObject(MyObject mo){
    ... Do the conversion
    return otherObject;
}

En el nuevo lugar de trabajo, el patrón es:

public void MyObject2OtherObject(MyObject mo, OtherObject oo){
    ... Do the conversion
}

Para mí es un poco maloliente, ya que me acostumbré a no cambiar los parámetros entrantes. ¿Es esta alteración del parámetro entrante un antipatrón o está bien? ¿Tiene algunos inconvenientes serios?

CsBalazsHungary
fuente
44
La respuesta correcta para esto será específica del idioma, ya que aquellos donde los parámetros de paso por valor los convierten efectivamente en variables locales.
Blrfl
1
El segundo patrón a veces se usa como una medida de eficiencia para reutilizar objetos. Suponiendo que ooes un objeto que se pasa al método, no un puntero que se establece en un nuevo objeto. ¿Es ese el caso aquí? Si esto es Java, probablemente lo sea, si es C ++ podría no serlo
Richard Tingle
3
Esto parece una optimización prematura, sí. Le recomiendo que use el primer formulario en general, pero si se encuentra con un cuello de botella en el rendimiento, puede usar el segundo formulario con un comentario para justificarlo . Un comentario evitaría que usted y los demás miren ese código y vuelvan a aparecer esta misma pregunta.
Keen
2
El segundo patrón es útil cuando la función también devuelve un valor, por ejemplo, éxito / fracaso. Pero no hay nada realmente 'malo' con eso. Realmente depende de dónde desea crear el objeto.
GrandmasterB
12
No nombraría las funciones que implementan estos dos patrones diferentes de la misma manera.
Casey

Respuestas:

70

No es un antipatrón, es una mala práctica.

La diferencia entre un antipatrón y una mera mala práctica está aquí: definición antipatrón .

El nuevo estilo de lugar de trabajo que muestra es una mala práctica , tiempos de vestigio o pre-OOP, de acuerdo con el Código de limpieza del tío Bob.

Los argumentos se interpretan más naturalmente como entradas a una función.

Cualquier cosa que lo obligue a verificar la firma de la función es equivalente a una doble toma. Es una ruptura cognitiva y debe evitarse. En los días previos a la programación orientada a objetos, a veces era necesario tener argumentos de salida. Sin embargo, gran parte de la necesidad de argumentos de salida desaparece en los lenguajes OO

Tulains Córdova
fuente
77
No veo por qué la definición que vinculaste impide que esto se considere un antipatrón.
Samuel Edwin Ward
77
Supongo que es porque la razón de que "estamos ahorrando CPU / memoria al no crear un nuevo objeto, en su lugar deberíamos reciclar uno que tenemos y pasarlo como un argumento" es obviamente incorrecto para todos con antecedentes serios de OOP que esto podría no constituyen un patrón jamás - así que no hay manera de que pudiéramos considerarlo un anti-patrón en la definición ...
vaxquis
1
¿Qué pasa con el caso cuando un parámetro de entrada es una gran colección de objetos que necesitan ser modificados?
Steve Chambers
Aunque también prefiero la opción 1, ¿qué OtherObjectpasa si es una interfaz? Solo la persona que llama (con suerte) sabe qué tipo de concreto quiere.
user949300
@SteveChambers Creo que, en este caso, el diseño de la clase o método es malo y debe ser refactorizado para evitarlo.
piotr.wittchen
16

Citando el famoso libro de Robert C. Martin "Código limpio":

Deben evitarse los argumentos de salida

Las funciones deben tener un pequeño número de argumentos.

El segundo patrón viola ambas reglas, particularmente el "argumento de salida". En este sentido, es peor que el primer patrón.

claasz
fuente
1
Diría que viola el primero, muchos argumentos significan código desordenado, pero creo que dos argumentos están totalmente bien. Creo que la regla sobre el código limpio significa que si necesita 5 o 6 datos entrantes, desea hacer demasiado en un solo método, por lo que debe refactorizar para aumentar la legibilidad del código y el alcance de OOP.
CsBalazsHungary
2
de todos modos sospecho que esto no es una ley estricta, sino una sugerencia fuerte. Sé que no funcionará con tipos primitivos, si se trata de un patrón extendido en un proyecto, un junior obtendría ideas para pasar un int y esperaría que se cambiara como Objetos.
CsBalazsHungary
27
Independientemente de cuán correcto sea el Código limpio, no creo que esta sea una respuesta valiosa sin explicar por qué se deben evitar. No son mandamientos que caen en una tableta de piedra, la comprensión es importante. Esta respuesta podría mejorarse proporcionando un resumen del razonamiento en el libro y una referencia de capítulo
Daenyth
3
Bueno, demonios, si algún tipo lo escribió en un libro, debe ser un buen consejo para cualquier situación concebible. No hay necesidad de pensar.
Casey
2
@emodendroket ¡Pero amigo, es el chico!
Pierre Arlaud
15

El segundo idioma puede ser más rápido, porque la persona que llama puede reutilizar una variable en un ciclo largo, en lugar de que cada iteración cree una nueva instancia.

Generalmente no lo usaría, pero por ejemplo. en la programación de juegos tiene su lugar. Por ejemplo, mire las muchas operaciones Vector3f de JavaMonkey que permiten pasar instancias que deberían modificarse y devolverse como resultado.

user470365
fuente
12
bueno, puedo estar de acuerdo si este es el cuello de botella, pero donde sea que trabajé, los cuellos de botella suelen ser algoritmos con baja eficiencia, no clonación o creación de objetos. Sé menos sobre desarrollo de juegos.
CsBalazsHungary
44
@CsBalazsHungary Creo que los problemas cuando la creación de objetos es una preocupación tienden a estar relacionados con las asignaciones de memoria y la recolección de basura, que probablemente sería el siguiente nivel de cuellos de botella después de la complejidad algorítmica (especialmente en un entorno de memoria limitada, por ejemplo, teléfonos inteligentes y similares) .
JAB
JMonkey también es donde he visto mucho esto, ya que a menudo es crítico para el rendimiento. Sin embargo
Richard Tingle
3
@JAB: El GC de JVM está ajustado específicamente para el caso de objetos efímeros. Las grandes cantidades de objetos de muy corta vida son triviales de recolectar, y en muchos casos toda su efímera generación podría recolectarse con un solo movimiento de puntero.
Phoshi
2
en realidad, si uno tiene que crear un objeto , no será efectivo para el rendimiento; si un bloque de código tiene que estar muy optimizado para la velocidad, debe usar primitivas y matrices nativas en su lugar; por eso, considero que la afirmación en esta respuesta no es válida.
vaxquis
10

No creo que sean dos códigos equivalentes. En el primer caso, debe crear el otherObject. Puede modificar la instancia existente en el segundo. Ambos tienen sus usos. El olor a código preferiría uno sobre otro.

Eufórico
fuente
Sé que no son equivalentes. Tienes razón, estamos persistiendo en estas cosas, por lo que marcaría la diferencia. Entonces, ¿estás diciendo que ninguno de ellos es antipatrón, es solo cuestión del caso de uso?
CsBalazsHungary
@CsBalazsHungary Yo diría que sí. A juzgar por el pequeño código que proporcionaste.
Eufórico
Supongo que se convierte en un problema más grave si tiene un parámetro primitivo entrante, que por supuesto no se actualizará, por lo que, como escribió @claasz: debe evitarse a menos que sea necesario.
CsBalazsHungary
1
@CsBalazsHungary En caso de argumento primitivo, la función ni siquiera funcionaría. Entonces tienes un problema peor que cambiar los argumentos.
Eufórico
Yo diría que un junior caería en la trampa de tratar de hacer de un tipo primitivo un parámetro de salida. Entonces diría que el primer convertidor debería preferirse si es posible.
CsBalazsHungary
7

Realmente depende del idioma.

En Java, la segunda forma puede ser un antipatrón, pero algunos lenguajes tratan el paso de parámetros de manera diferente. En Ada y VHDL por ejemplo, en lugar de pasar por valor o por referencia, los parámetros pueden tener modos in, outo in out.

Es un error modificar un inparámetro o leer un outparámetro, pero cualquier cambio en un in outparámetro se devuelve a la persona que llama.

Entonces, las dos formas en Ada (también son VHDL legales) son

function MyObject2OtherObject(mo : in MyObject) return OtherObject is
begin
    ... Do the conversion
    return otherObject;
end MyObject2OtherObject;

y

procedure MyObject2OtherObject(mo : in MyObject; oo : out OtherObject) is
begin
    ... Do the conversion
    oo := ... the conversion result;
end MyObject2OtherObject;

Ambos tienen sus usos; un procedimiento puede devolver múltiples valores en múltiples Outparámetros mientras que una función solo puede devolver un único resultado. Debido a que el propósito del segundo parámetro está claramente establecido en la declaración del procedimiento, no puede haber objeciones a este formulario. Tiendo a preferir la función de legibilidad. pero habrá casos en los que el procedimiento es mejor, por ejemplo, donde la persona que llama ya ha creado el objeto.

Brian Drummond
fuente
3

De hecho, parece maloliente, y sin ver más contexto es imposible decirlo con certeza. Podría haber dos razones para hacerlo, aunque existen alternativas para ambos.

Primero, es una forma concisa de implementar una conversión parcial, o dejar el resultado con valores predeterminados si la conversión falla. Es decir, podrías tener esto:

public void ConvertFoo(Foo from, Foo to) {
    if (can't convert) {
        return;
    }
    ...
}

Foo a;
Foo b = DefaultFoo();
ConvertFoo(a, b);
// If conversion fails, b is unchanged

Por supuesto, generalmente esto se maneja usando excepciones. Sin embargo, incluso si las excepciones deben evitarse por cualquier razón, hay una mejor manera de hacerlo: el patrón TryParse es una opción.

Otra razón es que podría ser por razones puramente consistentes, por ejemplo, es parte de una API pública donde este método se usa para todas las funciones de conversión por cualquier razón (como otras funciones de conversión que tienen múltiples salidas).

Java no es excelente para lidiar con múltiples salidas: no puede tener parámetros de solo salida como algunos lenguajes, o tener múltiples valores de retorno como otros, pero aún así, podría usar objetos de retorno.

La razón de la coherencia es poco convincente, pero lamentablemente puede ser la más común.

  • Quizás los policías de estilo en su lugar de trabajo (o su base de código) provienen de un entorno que no es Java y han sido reacios a cambiar.
  • Su código puede haber sido un puerto de un idioma donde este estilo es más idiomático.
  • Es posible que su organización necesite mantener la coherencia de la API en diferentes idiomas, y este fue el estilo de denominador común más bajo (es tonto pero sucede incluso para Google ).
  • O tal vez el estilo tenía más sentido en el pasado distante y se transformó en su forma actual (por ejemplo, podría haber sido el patrón TryParse pero algún predecesor bien intencionado eliminó el valor de retorno después de descubrir que nadie lo comprobó en absoluto).
congusbongus
fuente
2

La ventaja del segundo patrón es que obliga a la persona que llama a tomar posesión y responsabilidad del objeto creado. No hay duda de si el método creó o no el objeto o lo obtuvo de un grupo reutilizable. La persona que llama sabe que es responsable de la vida útil y la eliminación del nuevo objeto.

Las desventajas de este método son:

  1. No se puede utilizar como método de fábrica, la persona que llama debe conocer el subtipo exacto OtherObjectque necesita y construirlo con anticipación.
  2. No se puede usar con objetos que requieren parámetros en su constructor si esos parámetros provienen MyObject. OtherObjectdebe ser construible sin saberlo MyObject.

La respuesta se basa en mi experiencia en c #, espero que la lógica se traduzca a Java.

Rotem
fuente
Creo que con la responsabilidad que mencionas, también crea un ciclo de vida "más difícil de ver" de los objetos. En un sistema de método complejo, preferiría una modificación directa de un objeto en lugar de comenzar a examinar todo el método que estamos pasando.
CsBalazsHungary
Esto era lo que estaba pensando. Un tipo primitivo de inyección
Lyndon White
Otra ventaja es si OtherObject es abstracto o una interfaz. La función no tiene idea de qué tipo crear pero la persona que llama sí.
user949300
2

Dada la semántica laxa, myObjectToOtherObjectpodría significar igualmente que mueve algunos datos del primer objeto al segundo o que los convierte completamente de nuevo, el segundo método parece más adecuado.

Sin embargo, si el nombre del método fuera Convert(lo que debería ser si miramos la parte "... hacer la conversión"), diría que el segundo método no tendría ningún sentido. No convierte un valor en otro valor que ya existe, IMO.

guillaume31
fuente