2009-04-27 20 views
8

Me acabo de encontrar con un patrón que he visto antes, y quería obtener opiniones al respecto. El código en cuestión consiste en una interfaz como esta:Secret Handshake Anti-Pattern

public interface MyCrazyAnalyzer { 
    public void setOptions(AnalyzerOptions options); 
    public void setText(String text); 
    public void initialize(); 
    public int getOccurances(String query); 
} 

Y el uso previsto es la siguiente:

MyCrazyAnalyzer crazy = AnalyzerFactory.getAnalyzer(); 
crazy.setOptions(true); 
crazy.initialize(); 
Map<String, Integer> results = new HashMap<String, Integer>(); 
for(String item : items) { 
    crazy.setText(item); 
    results.put(item, crazy.getOccurances); 
} 

Hay razones para algo de esto. El setText (...) y getOccurances (...) están ahí porque hay varias consultas que podría querer hacer después de hacer el mismo análisis costoso de los datos, pero esto puede refactorizarse a una clase de resultado.

Por qué creo que esto es tan malo: la implementación almacena el estado de una manera que no está claramente indicada por la interfaz. También he visto algo similar que implica una interfaz que requiere llamar a "prepareResult", luego "getResult". Ahora, puedo pensar en un código bien diseñado que emplee algunas de estas características. La interfaz de Hadoop Mapper amplía JobConfigurable y Closeable, pero veo una gran diferencia porque es un marco que utiliza el código de usuario que implementa esas interfaces, frente a un servicio que podría tener múltiples implementaciones. Supongo que todo lo relacionado con la inclusión de un método "cerrado" al que se debe llamar está justificado, ya que no hay otra forma razonable de hacerlo. En algunos casos, como JDBC, esto es una consecuencia de una abstracción con goteras, pero en los dos códigos en los que estoy pensando, es una clara consecuencia de los programadores que rápidamente agregaron una interfaz a una clase de código de spaghetti para limpiarlo.

Mis preguntas son:

  1. ¿Todos están de acuerdo en que esto es una interfaz mal diseñado?
  2. ¿Se trata de un antipatrón descrito?
  3. ¿Este tipo de inicialización pertenece alguna vez en una interfaz?
  4. ¿Esto me parece incorrecto porque prefiero el estilo funcional y la inmutabilidad?

Si esto es lo suficientemente común como para merecer un nombre, sugiero el anti-patrón "Secret Handshake" para una interfaz que obliga a llamar a múltiples métodos en un orden particular cuando la interfaz no es intrínsecamente Una colección).

+0

Para ser honesto, este "patrón" me recuerda DataReaders en ADO.NET. Eso no significa que sea necesariamente una buena idea, pero se usa incluso en API serias. –

+0

Buena antipatrón. Si los objetos se hubieran inicializado así, podríamos especificar constructores en las interfaces. También me gusta el término "apretón de manos secreto". – CurtainDog

+0

Al menos es explícitamente malo, a diferencia de java.util.MessageFormat que usa el estado interno temporal durante llamadas de método único. –

Respuesta

17

Sí, es un antipatrón: Sequential coupling.

Refactorizaría en Options - pasó a la fábrica, y Results, regresó de un método analyseText().

+13

Excelente, ahora puedo poner "eliminar acoplamiento secuencial" en mis mensajes de registro en lugar de la menos educada "eliminar basura confusa". –

3
  1. Espero que AnalyzerFactory pase los parámetros necesarios y realice la construcción en sí; de lo contrario, ¿qué está haciendo exactamente?
  2. No estoy seguro si tiene un nombre, pero parece que debería :)
  3. Sí, ocasionalmente es conveniente (y el nivel de abstracción correcto) tener instaladores en su interfaz y esperar que las clases los llamen. Sugeriría que hacerlo requiere documentación extensa de ese hecho.
  4. No realmente, no. Una preferencia por la inmutabilidad es ciertamente una buena cosa, y el diseño basado en setter/bean puede ser la opción "correcta" algunas veces también, pero su ejemplo dado lo lleva demasiado lejos.
+0

+1 Porque me preguntaba qué estaba haciendo la fábrica también ... parece que solo se está utilizando como recurso global, otro no-no. – CurtainDog

3

No estoy seguro de si se trata de un anti-patrón descrito, pero estoy totalmente de acuerdo en que esta es una interfaz mal diseñada. Deja demasiadas oportunidades para el error y viola al menos un principio clave: hacer que su API sea difícil de utilizar.

Además del mal uso, esta API también puede llevar a errores difíciles de depurar si varios subprocesos hacen uso de la misma instancia.

Joshua Bloch tiene realmente un excelente presentation (36m16s y 40m30s) en el diseño de la API y aborda esta como una de las características de una API mal diseñada.

0

No puedo ver nada malo aquí. setText() prepara el escenario; después de eso, tiene una o más llamadas al getOccurances(). Como el setText() es muy caro, no se me ocurre ninguna otra forma de hacerlo.

getOccurances(text, query) arreglaría el "apretón de manos secreto" a un costo de rendimiento tremendo. Puede tratar de guardar el texto en caché en getOccurances() y solo actualizar sus cachés internos cuando el texto cambie, pero eso comienza a parecerse más y más a sacrificar algún principio de OO. Si una regla no tiene sentido, entonces no la aplique. Los desarrolladores de software tienen cerebro por una razón.

+0

Como sugiere Douglas, es posible refactorizar este tipo de código en algo que devuelva un objeto Result. –

0

Una posible solución: utilice el cambio fluido. Eso evita que una clase contenga métodos que necesitan llamar en un cierto orden. Se parece mucho al patrón de generador que asegura que no lea objetos que aún están en el medio de ser poblados.

Cuestiones relacionadas