¿Hay algo intrínsecamente incorrecto en las cadenas de invocación de objetos largos?

Question

He organizado mi código para que los datos estén organizados jerárquicamente. Me encuentro trepando por el árbol utilizando código como el siguiente:

File clientFolder = task.getActionPlan().getClientFile().getClient().getDocumentsFolder(); 

Ahora, ya que no estoy perforando hacia abajo en el objeto de tarea, estoy perforar hasta sus padres, no creo que yo' Estoy perdiendo cualquier cosa en términos de encapsulación, pero en mi mente hay una bandera que me dice que hay algo sucio en hacerlo de esta manera.

¿Pensamientos?

Answer 1

Lea en The Law of Demeter.

Answer 2

La bandera más grande del mundo.

No se puede verificar fácilmente si alguna de esas invokations devuelve un objeto nulo, lo que hace que el seguimiento de cualquier tipo de error sea casi imposible.

getClientFile() puede devolver el valor nulo y luego getClient() fallará y, cuando esté detectando esto, suponiendo que está intentando capturarlo, no tendrá ni idea de cuál falló.

Answer 3

¿Cuán probable es obtener nulos o resultados no válidos? Ese código depende del retorno exitoso de muchas funciones y podría ser más difícil resolver errores como la excepción del puntero nulo.

También es malo para el depurador: menos informativo ya que tiene que ejecutar las funciones en lugar de observar solo algunas variables locales, y es incómodo pasar a las funciones posteriores de la cadena.

Answer 4

Sí. No es una mejor práctica. Por un lado, si hay un error, es más difícil encontrarlo. Por ejemplo, su controlador de excepción puede mostrar un seguimiento de pila que muestre que tiene una NullReferenceException en la línea 121, pero ¿cuál de estos métodos devuelve nulo? Tendrás que profundizar en el código para averiguarlo.

Answer 5

Esta es una pregunta subjetiva, pero no creo que haya ningún problema hasta cierto punto. Por ejemplo, si la cadena se extiende más allá del área de lectura legible del editor, entonces debe introducir algunos locales. Por ejemplo, en mi navegador no puedo ver las últimas 3 llamadas, así que no tengo idea de lo que estás haciendo :).

Answer 6

Bueno, eso depende. No deberías tener que alcanzar a través de un objeto como ese. Si controla las implementaciones de esos métodos, recomiendo refactorizar para que no tenga que hacer eso.

De lo contrario, no veo ningún daño en hacer lo que estás haciendo. Ciertamente es mejor que

ActionPlan AP = task.getActionPlan(); 
ClientFile CF = AP.getClientFile(); 
Client C = CF.getClient(); 
DocFolder DF = C.getDocumentsFolder(); 

Answer 7

Bueno, cada indirección agrega un punto donde podría salir mal.

En particular, cualquiera de los métodos de esta cadena podría devolver un valor nulo (en teoría, en su caso podría tener métodos que no pueden hacer eso), y cuando eso suceda, sabrá que pasó a uno de esos métodos, pero no cuál.

Entonces, si hay alguna posibilidad de que alguno de los métodos devuelva un valor nulo, al menos dividiría la cadena en esos puntos, y almacenaría en variables intermedias, y la dividiría en líneas individuales, para que un informe de fallas me daría un número de línea para mirar.

Aparte de eso no puedo ver ningún problema obvio con él.Si tiene, o puede hacer, garantías de que la referencia nula no será un problema, haría lo que quiera.

¿Qué hay de la legibilidad? ¿Sería más claro si agregó variables con nombre? Siempre escriba el código como si fuera la intención de que un programador compañero lo leyera, y solo un compilador lo interpretaría incidentalmente.

En este caso tendría que leer la cadena de llamadas de método y descubrir ... bien, obtiene un documento, es el documento de un cliente, el cliente proviene de un ... archivo ... derecha, y el archivo es de un plan de acción, etc. cadenas largas puede que sea menos legible que, por ejemplo, esto:

ActionPlan taskPlan = task.GetActionPlan(); 
ClientFile clientFileOfTaskPlan = taskPlan.GetClientFile(); 
Client clientOfTaskPlan = clientFileOfTaskPlan.GetClient(); 
File clientFolder = clientOfTaskPlan.getDocumentsFolder(); 

supongo que se trata de una opinión personal sobre este asunto.

Answer 8

No está mal como tal, pero puede tener problemas para leer esto en 6 meses. O un compañero de trabajo podría tener problemas para mantener/escribir código, porque la cadena de tus objetos es bastante ... larga.

Y reconozco que no tiene que introducir variables en su código. Entonces los objetos sí saben todo lo que necesitan para saltar de un método a otro. (Aquí surge la pregunta si no has sobreiniciado un poco, ¿pero quién soy yo?)

Me gustaría introducir un tipo de "métodos de conveniencia". Imagine que tiene un método en su "tarea" - objeto algo así como

task.getClientFromActionPlan(); 

A continuación, seguramente podría utilizar

task.getClientFromActionPlan().getDocumentsFolder(); 

Mucho más fácil de leer y en caso de que estos "métodos de conveniencia" a la derecha (es decir, para cadenas pesadas de objetos usados), mucho menos para escribir ;-).

Edith dice: Estos métodos de conveniencia que sugiero a menudo contienen Nullpointer-checking cuando los escribo. De esta forma, incluso puede lanzar los Nullpointers con buenos mensajes de error (es decir, "ActionPlan fue nulo al tratar de recuperar el cliente de él").

Answer 9

discusión relacionada:

Function Chaining - How many is too many?

Answer 10

Esta cuestión está muy cerca de https://stackoverflow.com/questions/154864/function-chaining-how-many-is-too-many#155407

En general, parece que las personas están de acuerdo en que las cadenas demasiado largas no son buenas y que deben atenerse a uno o dos llamadas encadenadas como máximo.

Aunque escuché que los fanáticos de Python consideran que encadenarse es muy divertido. Eso podría ser solo un rumor ... :-)

Answer 11

En primer lugar, el código de apilamiento de esa manera puede hacer que resulte molesto analizar NullPointerExceptions y verificar las referencias al entrar en un depurador.

Aparte de eso, creo que todo se reduce a esto: ¿La persona que llama necesita tener todo ese conocimiento?

Quizás su funcionalidad podría hacerse más genérica; el archivo se puede pasar como un parámetro en su lugar. O, ¿quizás el ActionPlan ni siquiera debería revelar que su implementación se basa en un ClientFile?

Answer 12

Dependiendo de su objetivo final, es probable que desee utilizar The Principal of Least Knowledge para evitar el acoplamiento pesado y que le cueste al final. Como le gusta a la cabeza en primer lugar ... "Solo habla con tus amigos".

Answer 13

Acepto el cartel que menciona la Ley de Demeter. Lo que está haciendo es crear dependencias innecesarias en las implementaciones de muchas de estas clases, y en la estructura de la jerarquía misma. Hará que sea muy difícil probar el código de forma aislada, ya que tendrá que inicializar una docena de otros objetos solo para obtener una instancia de trabajo de la clase que desea probar.

Answer 14

Otro subproducto importante del encadenamiento es el rendimiento.

No es un gran problema en la mayoría de los casos, pero especialmente en un ciclo se puede ver un aumento razonable en el rendimiento al reducir la indirección.

Encadenamiento también hace que sea más difícil estimar el rendimiento, no se puede decir cuál de esos métodos puede o no hacer algo complejo.

Answer 15

la bandera es de color rojo, y dice dos cosas en negrita:

• a seguir la cadena es necesario que el código de llamada para saber toda la estructura de árbol, que no es una buena encapsulación, y
• si la jerarquía cambie alguna vez va a tener una gran cantidad de código para editar

y una cosa entre paréntesis:

• utilizar una propiedad, es decir, en lugar de task.ActionPlan task.getActionPlan()

una mejor solución podría ser - si se asume que necesita para exponer todas las propiedades de los padres hasta el árbol en el nivel infantil - ir adelante y poner en práctica las propiedades directas sobre los hijos, es decir

File clientFolder = task.DocumentsFolder; 

esto por lo menos ocultar la estructura de árbol del código de llamada. Internamente las propiedades pueden ser:

class Task { 
    public File DocumentsFolder { 
     get { return ActionPlan.DocumentsFolder; } 
    } 
    ... 
} 
class ActionPlan { 
    public File DocumentsFolder { 
     get { return ClientFile.DocumentsFolder: } 
    } 
    ... 
} 
class ClientFile { 
    public File DocumentsFolder { 
     get { return Client.DocumentsFolder; } 
    } 
    ... 
} 
class Client { 
    public File DocumentsFolder { 
     get { return ...; } //whatever it really is 
    } 
    ... 
} 

pero si los cambios en la estructura de árbol en el futuro sólo se tendrán que cambiar las funciones de acceso en las clases involucradas en el árbol, y no todos los lugares donde se ha llamado en la cadena .

[además de que será más fácil para atrapar e informar adecuadamente nulos en las funciones de propiedad, que fue omitido en el ejemplo anterior]

Answer 16

Qué actual. Voy a escribir una publicación en mi blog esta noche sobre este olor, Message Chains, frente a su inversa, Middle Man.

De todos modos, una pregunta más profunda es por qué tiene métodos de "obtener" en lo que parece ser un objeto de dominio. Si sigue de cerca los contornos del problema, descubrirá que no tiene sentido decirle a una tarea que obtenga algo, o que lo que está haciendo realmente no es una cuestión de lógica empresarial, como prepararse para la visualización de la interfaz de usuario, persistencia, reconstrucción del objeto, etc.

En este último caso, los métodos "get" están bien siempre y cuando sean used by authorized classes.La forma en que aplicas esa política depende de la plataforma y del proceso.

Por lo tanto, en el caso donde los métodos de "obtener" se consideran correctos, aún debe enfrentar el problema. Y desafortunadamente, creo que depende de la clase que está navegando por la cadena. Si es apropiado que esa clase se acople a la estructura (por ejemplo, una fábrica), entonces déjalo. De lo contrario, debe intentar Hide Delegate.

Editar: click here for my post.

Answer 17

Getters and setters are evil. En general, evite que un objeto haga algo con él. En lugar de delegar la tarea en sí.

En lugar de

Object a = b.getA(); 
doSomething(a); 

hacer

b.doSomething(); 

Al igual que con todos los principios de diseño, que no siguen esta ciegamente. Nunca he sido capaz de escribir algo remotamente complicado sin getters y setters, pero es una buena guía. Si tiene muchos buscadores y setters, probablemente significa que lo está haciendo mal.

Answer 18

Diría La ley de Demeter, también.

Y añada un artículo sobre Tell, Don't Ask

Answer 19

¿Está realista va a utilizar siempre todas y cada una de esas funciones de manera independiente? ¿Por qué no simplemente hacer que la tarea tenga un método GetDocumentsFolder() que hace todo el trabajo sucio de llamar a todos esos métodos por usted? Entonces puede hacer que todo el trabajo sucio sea una verificación nula de todo sin desmenuzar su código en lugares donde no es necesario deshacerlo.

Answer 20

OK, como otros señalan que el código no es excelente porque está bloqueando el código de una jerarquía específica. Puede presentar problemas de depuración, y no es agradable de leer, pero el punto principal es que el código que toma una tarea sabe demasiado sobre cómo atravesar para obtener algo de carpeta. De dólares a donuts, alguien querrá insertar algo en el medio. (todas las tareas están en una lista de tareas, etc.)

Salir en una extremidad, ¿son todas estas clases solo nombres especiales para la misma cosa? es decir, ¿son jerárquicos, pero cada nivel tiene quizás algunas propiedades adicionales?

Por lo tanto, desde un ángulo diferente, voy a simplificar a una enumeración y una interfaz, donde las clases secundarias delegan la cadena si no son lo que se solicita. Por el bien de la discusión, los estoy llamando carpetas.

enum FolderType { ActionPlan, ClientFile, Client, etc } 

interface IFolder 
{ 
    IFolder FindTypeViaParent(FolderType folderType) 
} 

y cada clase que implementa iFolder probablemente sólo hace

IFolder FindTypeViaParent(FolderType folderType) 
{ 
    if(myFolderType == folderType) 
     return this; 

    if(parent == null) 
     return null; 

    IFolder parentIFolder = (IFolder)parent; 
    return parentIFolder.FindTypeViaParent(folderType) 
} 

Una variación es hacer que la interfaz de iFolder:

interface IFolder 
{ 
    FolderType FolderType { get; } 
    IFolder Parent { get; } 
} 

Esto le permite externalizar el código de recorrido. Sin embargo, esto les quita el control de las clases (tal vez tengan padres múltiples) y expone la implementación. Bueno y malo.

[divagaciones]

A primera vista esto parece ser una jerarquía bastante caros de instalar. ¿Necesito instanciar de arriba hacia abajo todo el tiempo? es decir, si algo solo necesita una tarea, ¿tiene que instanciar todo de abajo hacia arriba para asegurarse de que todos esos punteros funcionan? Incluso si es una carga lenta, ¿necesito subir la jerarquía solo para obtener la raíz?

Por otra parte, ¿la jerarquía es realmente una parte de la identidad del objeto? Si no es así, quizás podría externalizar la jerarquía como un árbol n-ario.

Como nota al margen, es posible que desee considerar el concepto de agregación DDD (Domain Driven Design) y determinar quiénes son los principales jugadores. ¿Cuál es el último objeto propietario que es responsable? p.ej. ruedas de un auto. En un diseño que modela un automóvil, las ruedas también son objetos, pero son propiedad del automóvil.

Quizás le funcione, quizás no. Como dije, esto es solo un tiro en la oscuridad.

Answer 21

Por otro lado, el Law of Demeter isn't universally applicable, ni una regla dura (arrr, it be more of a guideline!).