Cuánta lógica en Getters

Mis compañeros de trabajo me dicen que debería haber la menor lógica posible en captadores y colocadores.

Sin embargo, estoy convencido de que se pueden ocultar muchas cosas en getters y setters para proteger a los usuarios / programadores de los detalles de implementación.

Un ejemplo de lo que hago:

public List<Stuff> getStuff()
{
   if (stuff == null || cacheInvalid())
   {
       stuff = getStuffFromDatabase();
   }
   return stuff;
}

Un ejemplo de cómo el trabajo me dice que haga cosas (citan 'Código limpio' del tío Bob):

public List<Stuff> getStuff()
{
    return stuff;
}

public void loadStuff()
{
    stuff = getStuffFromDatabase();
}

¿Cuánta lógica es apropiada en un setter / getter? ¿De qué sirven los getters y setters vacíos, excepto una violación de la ocultación de datos?

La forma en que el trabajo te dice que hagas las cosas es poco convincente.

Como regla general, la forma en que hago las cosas es la siguiente: si obtener el material es computacionalmente barato (o si lo más probable es que se encuentre en el caché), entonces su estilo de getStuff () está bien. Si se sabe que obtener el material es computacionalmente costoso, tan costoso que anunciar su costo es necesario en la interfaz, entonces no lo llamaría getStuff (), lo llamaría CalculateStuff () o algo así, para indicar que Habrá algo de trabajo por hacer.

En cualquier caso, la forma en que el trabajo le dice que haga las cosas es poco convincente, porque getStuff () explotará si no se ha llamado a loadStuff () por adelantado, por lo que esencialmente quieren que complique su interfaz al introducir la complejidad del orden de operaciones lo. El orden de las operaciones se trata básicamente del peor tipo de complejidad que se me ocurre.

La lógica en getters está perfectamente bien.

Pero obtener datos de una base de datos es mucho más que "lógica". Implica una serie de operaciones muy caras en las que muchas cosas pueden salir mal y de una manera no determinista. Dudaría en hacer eso implícitamente en un captador.

Por otro lado, la mayoría de los ORM admiten la carga lenta de colecciones, que es básicamente exactamente lo que estás haciendo.

Creo que de acuerdo con 'Clean Code' debería dividirse tanto como sea posible, en algo como:

public List<Stuff> getStuff() {
   if (hasStuff()) {
       return stuff;
   }
   loadStuff();
   return stuff;
}

private boolean hasStuff() {
    if (stuff == null) {
       return false;
    }
    if (cacheInvalid()) {
       return false;        
    }
    return true;
} 

private void loadStuff() {
    stuff = getStuffFromDatabase();
}

Por supuesto, esto no tiene sentido, dado que la hermosa forma, que usted escribió, hace lo correcto con una fracción de código que cualquiera entiende de un vistazo:

public List<Stuff> getStuff() {
   if (stuff == null || cacheInvalid()) {
       stuff = getStuffFromDatabase();
   }
   return stuff;
}

No debería ser el dolor de cabeza de la persona que llama cómo se pone el material debajo del capó, y particularmente no debería ser el dolor de cabeza de la persona que llama recordar recordar las cosas en un "orden correcto" arbitrario.

Me dicen que debería haber la menor lógica posible en getters y setters.

Es necesario que haya tanta lógica como sea necesaria para satisfacer las necesidades de la clase. Mi preferencia personal es la menor cantidad posible, pero cuando se mantiene el código, por lo general, debe abandonar la interfaz original con los captadores / establecedores existentes, pero poner mucha lógica en ellos para corregir la lógica comercial más reciente (como un ejemplo, "clientes "getter en un entorno posterior al 911 tiene que cumplir con las normativas " conozca a su cliente "y OFAC , junto con una política de la compañía que prohíbe la aparición de clientes de ciertos países [como Cuba o Irán]).

En su ejemplo, prefiero el suyo y no me gusta la muestra de "tío Bob", ya que la versión "tío Bob" requiere que los usuarios / mantenedores recuerden llamar loadStuff()antes de llamar getStuff(): esta es una receta para el desastre si alguno de sus mantenedores se olvida (o peor, nunca lo supe). La mayoría de los lugares en los que he trabajado en la última década todavía usan código que tiene más de una década, por lo que la facilidad de mantenimiento es un factor crítico a tener en cuenta.

Tienes razón, tus colegas están equivocados.

Olvídese de las reglas generales de todos sobre lo que un método get debería o no debería hacer. Una clase debe presentar una abstracción de algo. Tu clase tiene legibilidad stuff. En Java es convencional usar métodos 'get' para leer propiedades. Se han escrito miles de millones de líneas de marcos esperando leer stuffal llamar getStuff. Si nombra su función fetchStuffo cualquier otra cosa getStuff, su clase será incompatible con todos esos marcos.

Puede señalarlos a Hibernate, donde 'getStuff ()' puede hacer cosas muy complicadas y arroja una RuntimeException en caso de falla.

Parece que esto podría ser un poco un debate purista versus de aplicación que podría verse afectado por cómo prefiere controlar los nombres de las funciones. Desde el punto de vista aplicado, preferiría ver:

List<String> names = clientRoster.getNames();
List<String> emails = clientRoster.getEmails();

Opuesto a:

myObject.load();
List<String> names = clientRoster.getNames();
List<String> emails = clientRoster.getEmails();

O peor aún:

myObject.loadNames();
List<String> names = clientRoster.getNames();
myOjbect.loadEmails();
List<String> emails = clientRoster.getEmails();

Lo que tiende a hacer que otros códigos sean mucho más redundantes y difíciles de leer porque tienes que comenzar a leer todas las llamadas similares. Además, llamar a las funciones del cargador o similares interrumpe el propósito de incluso usar OOP en el sentido de que ya no se lo abstrae de los detalles de implementación del objeto con el que está trabajando. Si tiene un clientRosterobjeto, no debería tener que preocuparse por cómo getNamesfunciona, como lo haría si tuviera que llamar a loadNames, solo debe saber que getNamesle da un a List<String>con los nombres de los clientes.

Por lo tanto, parece que el problema es más sobre la semántica y el mejor nombre para que la función obtenga los datos. Si la empresa (y otras) tiene un problema con el prefijo gety set, entonces, ¿qué tal llamar a la función algo así retrieveNames? Dice lo que está sucediendo, pero no implica que la operación sea instantánea como se podría esperar de un getmétodo.

En términos de lógica en un método de acceso, manténgalo al mínimo ya que generalmente se supone que son instantáneos y solo se produce una interacción nominal con la variable. Sin embargo, eso también se aplica generalmente a tipos simples, tipos de datos complejos (es decir List). Me resulta más difícil encapsular adecuadamente en una propiedad y, en general, utilizo otros métodos para interactuar con ellos en lugar de un mutador y un accesor estrictos.

Llamar a un captador debería exhibir el mismo comportamiento que leer un campo:

• Debería ser barato recuperar el valor
• Si establece un valor con el setter y luego lo lee con el getter, el valor debería ser el mismo
• Obtener el valor no debería tener efectos secundarios
• No debe arrojar una excepción.

Un captador que invoca otras propiedades y métodos para calcular su propio valor también implica una dependencia. Por ejemplo, si su propiedad tiene que ser capaz de computarse a sí misma, y ​​hacerlo requiere que se establezca otro miembro, entonces debe preocuparse por referencias nulas accidentales si se accede a su propiedad en el código de inicialización donde no todos los miembros están necesariamente establecidos.

Eso no significa 'nunca acceder a otro miembro que no sea el campo de respaldo de propiedades dentro del captador', solo significa prestar atención a lo que está implicando sobre el estado requerido del objeto, y si eso coincide con el contexto que espera Esta propiedad para acceder en.

Sin embargo, en los dos ejemplos concretos que dio, la razón por la que elegiría uno sobre el otro es completamente diferente. Su getter se inicializa en el primer acceso, por ejemplo, Inicialización diferida . Se supone que el segundo ejemplo se inicializa en algún punto anterior, por ejemplo, Inicialización explícita .

Cuando ocurre exactamente la inicialización puede o no ser importante.

Por ejemplo, puede ser muy lento y debe hacerse durante un paso de carga en el que el usuario espera un retraso, en lugar de un rendimiento inesperado cuando el usuario activa el acceso por primera vez (es decir, hace clic con el botón derecho del usuario, aparece el menú contextual, usuario ya ha hecho clic derecho nuevamente).

Además, a veces hay un punto obvio en la ejecución donde ocurre todo lo que puede afectar / ensuciar el valor de la propiedad en caché. Incluso puede verificar que ninguna de las dependencias cambie y lanzar excepciones más adelante. En esta situación, tiene sentido también almacenar en caché el valor en ese punto, incluso si no es particularmente costoso de calcular, solo para evitar que la ejecución del código sea más compleja y difícil de seguir mentalmente.

Dicho esto, Lazy Initialization tiene mucho sentido en muchas otras situaciones. Entonces, como sucede a menudo en la programación, es difícil reducirlo a una regla, todo se reduce al código concreto.

Simplemente hazlo como dijo @MikeNakis ... Si solo obtienes las cosas, entonces está bien ... Si haces otra cosa, crea una nueva función que haga el trabajo y hazla pública.

Si su propiedad / función está haciendo solo lo que dice su nombre, entonces no queda mucho margen para complicaciones. La cohesión es clave IMO

Personalmente, expondría el requisito de Stuff a través de un parámetro en el constructor, y permitiría que cualquier clase que esté instanciando cosas haga el trabajo de averiguar de dónde debería venir. Si el material es nulo, debería devolver nulo. Prefiero no intentar soluciones inteligentes como la original del OP porque es una manera fácil de ocultar errores en el interior de su implementación, donde no es del todo obvio qué podría salir mal cuando algo se rompe.

Aquí hay cuestiones más importantes que la "adecuación", y usted debe basar su decisión en ellas . Principalmente, la gran decisión aquí es si desea permitir que las personas omitan el caché o no.

1. Primero, piense si hay una manera de reorganizar su código para que todas las llamadas de carga necesarias y la gestión de caché se realicen en el constructor / inicializador. Si esto es posible, puede crear una clase cuya invariante le permita hacerlo al captador simple de la parte 2 con la seguridad del captador complejo de la parte 1. (Un escenario de ganar-ganar)

2. Si no puede crear una clase de este tipo, decida si tiene un compromiso y necesita decidir si desea permitir al consumidor omitir el código de verificación de caché o no.

   1. Si es importante que el consumidor nunca omita la verificación de la memoria caché y no le importen las penalizaciones de rendimiento, entonces acople la verificación dentro del getter y haga imposible que el consumidor haga lo incorrecto.

   2. Si está bien omitir la comprobación de la memoria caché o es muy importante que tenga garantizado el rendimiento O (1) en el captador, utilice llamadas separadas.

Como ya habrás notado, no soy un gran admirador de la filosofía del "código limpio", "divide todo en pequeñas funciones". Si tiene un montón de funciones ortogonales que se pueden invocar en cualquier orden, dividirlas le dará más poder expresivo a bajo costo. Sin embargo, si sus funciones tienen dependencias de orden (o solo son realmente útiles en un orden particular), dividirlas solo aumenta la cantidad de formas en que puede hacer cosas incorrectas, a la vez que agrega poco beneficio.

En mi opinión, Getters no debería tener mucha lógica en ellos. No deberían tener efectos secundarios y nunca debería obtener una excepción de ellos. A menos, por supuesto, que sepas lo que estás haciendo. La mayoría de mis captadores no tienen lógica y simplemente van a un campo. Pero la notable excepción a eso fue con una API pública que debía ser lo más simple posible de usar. Así que tuve un captador que fallaría si no se hubiera llamado a otro captador. ¿La solución? Una línea de código como var throwaway=MyGetter;en el getter que dependía de ello. No estoy orgulloso de ello, pero todavía no veo una forma más limpia de hacerlo.

Esto parece una lectura de caché con carga diferida. Como otros han señalado, la verificación y la carga pueden pertenecer a otros métodos. Es posible que la carga deba sincronizarse para que no se carguen veinte subprocesos todos al mismo tiempo.

Puede ser apropiado usar un nombre getCachedStuff()para el getter ya que no tendrá un tiempo de ejecución constante.

Dependiendo de cómo funciona la cacheInvalid()rutina, la verificación de nulo puede no ser necesaria. No esperaría que el caché sea válido a menos que stuffse haya llenado de la base de datos.

La lógica principal que esperaría ver en los captadores que devuelven una lista es lógica para asegurarse de que la lista no se pueda modificar. Tal como está, ambos ejemplos pueden romper la encapsulación

algo como:

public List<Stuff> getStuff()
{
    return Collections.unmodifiableList(stuff);
}

En cuanto al almacenamiento en caché en el getter, creo que esto estaría bien, pero podría estar tentado a mover la lógica de caché si la construcción de la caché tomó un tiempo significativo. es decir, depende

Dependiendo del caso de uso exacto, me gusta separar mi almacenamiento en caché en una clase separada. Haga una StuffGetterinterfaz, implemente una StuffComputerque haga los cálculos y envuélvala dentro de un objeto de StuffCacher, que es responsable de acceder a la memoria caché o reenviar llamadas a la StuffComputerque se envuelve.

interface StuffGetter {
     public List<Stuff> getStuff();
}

class StuffComputer implements StuffGetter {
     public List<Stuff> getStuff() {
         getStuffFromDatabase()
     }
}

class StuffCacher implements StuffGetter {
     private stuffComputer; // DI this
     private Cache<List<Stuff>> cache = new Cache<>();

     public List<Stuff> getStuff() {
         if cache.hasStuff() {
             return cache.getStuff();
         }

         List<Stuffs> stuffs = stuffComputer.getStuff();
         cache.store(stuffs);
         return stuffs;
     }
}

Este diseño le permite agregar fácilmente el almacenamiento en caché, eliminar el almacenamiento en caché, cambiar la lógica de derivación subyacente (por ejemplo, acceder a una base de datos frente a devolver datos simulados), etc. Es un poco extenso, pero vale la pena para proyectos suficientemente avanzados.

En mi humilde opinión, es muy simple si utiliza un diseño por contrato. Decida qué debe proporcionar su captador y solo codifique en consecuencia (código simple o alguna lógica compleja que pueda estar involucrada o delegada en algún lugar).