¿Agregar un tipo de retorno a un método de actualización viola el "Principio de responsabilidad única"?

Tengo un método que actualiza los datos de los empleados en la base de datos. La Employeeclase es inmutable, por lo que "actualizar" el objeto significa en realidad crear una instancia de un nuevo objeto.

Quiero que el Updatemétodo devuelva una nueva instancia de Employeecon los datos actualizados, pero como ahora puedo decir que la responsabilidad del método es actualizar los datos de los empleados y obtener de la base de datos un nuevo Employeeobjeto , ¿viola el Principio de responsabilidad única ?

El registro DB en sí mismo se actualiza. Luego, se crea una instancia de un nuevo objeto para representar este registro.

Al igual que con cualquier regla, creo que lo importante aquí es considerar el propósito de la regla, el espíritu, y no verse atrapado en analizar exactamente cómo se redactó la regla en algún libro de texto y cómo aplicarla en este caso. No necesitamos abordar esto como abogados. El propósito de las reglas es ayudarnos a escribir mejores programas. No es que el propósito de escribir programas sea mantener las reglas.

El propósito de la regla de responsabilidad única es hacer que los programas sean más fáciles de entender y mantener haciendo que cada función haga una cosa coherente y autónoma.

Por ejemplo, una vez escribí una función a la que llamé algo así como "checkOrderStatus", que determinaba si un pedido estaba pendiente, enviado, pedido en espera, lo que sea, y devolvió un código que indica cuál. Luego apareció otro programador y modificó esta función para actualizar también la cantidad disponible cuando se envió el pedido. Esto violó severamente el principio de responsabilidad única. Otro programador que lea ese código más tarde vería el nombre de la función, vería cómo se utilizó el valor de retorno y bien podría nunca sospechar que realizó una actualización de la base de datos. Alguien que necesitara obtener el estado del pedido sin actualizar la cantidad disponible estaría en una posición incómoda: ¿debería escribir una nueva función que duplique la parte del estado del pedido? ¿Agregar una bandera para indicarle si se debe hacer la actualización de db? Etc.

Por otro lado, no quisiera señalar lo que constituye "dos cosas". Recientemente escribí una función que envía información del cliente desde nuestro sistema al sistema de nuestro cliente. Esa función realiza un reformateo de los datos para cumplir con sus requisitos. Por ejemplo, tenemos algunos campos que pueden ser nulos en nuestra base de datos, pero no permiten nulos, por lo que tenemos que completar un texto falso, "no especificado" o me olvido de las palabras exactas. Podría decirse que esta función está haciendo dos cosas: formatear los datos Y enviarlos. Pero deliberadamente pongo esto en una sola función en lugar de tener "reformatear" y "enviar" porque no quiero enviar nunca sin reformatear. No quiero que alguien escriba una nueva llamada y no me dé cuenta de que tiene que llamar a reformatear y luego enviar.

En su caso, actualizar la base de datos y devolver una imagen del registro escrito parecen dos cosas que bien podrían ir juntas de manera lógica e inevitable. No conozco los detalles de su aplicación, así que no puedo decir definitivamente si es una buena idea o no, pero parece plausible.

Si está creando un objeto en la memoria que contiene todos los datos para el registro, realiza las llamadas a la base de datos para escribir esto y luego devuelve el objeto, esto tiene mucho sentido. Tienes el objeto en tus manos. ¿Por qué no simplemente devolverlo? Si no devolvió el objeto, ¿cómo lo obtendría la persona que llama? ¿Tendría que leer la base de datos para obtener el objeto que acaba de escribir? Eso parece bastante ineficiente. ¿Cómo encontraría el registro? ¿Conoces la clave primaria? Si alguien declara que es "legal" que la función de escritura devuelva la clave primaria para que pueda volver a leer el registro, ¿por qué no simplemente devolver todo el registro para que no tenga que hacerlo? ¿Cual es la diferencia?

Por otro lado, si crear el objeto es un trabajo bastante distinto de escribir el registro de la base de datos, y una persona que llama podría querer escribir pero no crear el objeto, entonces esto podría ser un desperdicio. Si una persona que llama puede querer el objeto pero no hace la escritura, entonces tendría que proporcionar otra forma de obtener el objeto, lo que podría significar escribir código redundante.

Pero creo que el escenario 1 es más probable, así que diría, probablemente no hay problema.

El concepto del SRP es evitar que los módulos hagan 2 cosas diferentes cuando los hace individualmente para un mejor mantenimiento y menos espagueti a tiempo. Como dice el SRP "una razón para cambiar".

En su caso, si tenía una rutina que "actualiza y devuelve el objeto actualizado", todavía está cambiando el objeto una vez, lo que le da 1 razón para cambiar. Que devuelva el objeto de vuelta no es ni aquí ni allá, todavía está operando en ese único objeto. El código tiene la responsabilidad de una y solo una cosa.

El SRP no se trata realmente de tratar de reducir todas las operaciones a una sola llamada, sino de reducir lo que está operando y cómo lo está haciendo. Entonces, una única actualización que devuelve el objeto actualizado está bien.

Como siempre, esta es una cuestión de grado. El SRP debería evitar que escriba un método que recupere un registro de una base de datos externa, realice una transformación rápida de Fourier y actualice un registro de estadísticas globales con el resultado. Creo que casi todos estarían de acuerdo en que estas cosas deberían hacerse por diferentes métodos. Postulando un solo responsabilidad para cada método es simplemente la forma más económica y memorable de hacer ese punto.

En el otro extremo del espectro hay métodos que producen información sobre el estado de un objeto. Lo tipicoisActive tiene que dar esta información como su única responsabilidad. Probablemente todos estén de acuerdo en que esto está bien.

Ahora, algunos extienden el principio tanto que consideran que devolver una bandera de éxito es una responsabilidad diferente de realizar la acción cuyo éxito se informa. Bajo una interpretación extremadamente estricta, esto es cierto, pero como la alternativa sería tener que llamar a un segundo método para obtener el estado de éxito, lo que complica a la persona que llama, muchos programadores están perfectamente de acuerdo con devolver códigos de éxito de un método con efectos secundarios.

Devolver el nuevo objeto es un paso más en el camino hacia múltiples responsabilidades. Requerir que la persona que llama haga una segunda llamada para un objeto completo es un poco más razonable que requerir una segunda llamada solo para ver si la primera tuvo éxito o no. Aún así, muchos programadores considerarían devolver el resultado de una actualización perfectamente bien. Si bien esto puede interpretarse como dos responsabilidades ligeramente diferentes, ciertamente no es uno de los abusos atroces que inspiraron el principio.

¿Viola el principio de responsabilidad única?

No necesariamente. En todo caso, esto viola el principio de la separación comando-consulta .

La responsabilidad es

actualizar los datos del empleado

con el entendimiento implícito de que esta responsabilidad incluye el estado de la operación; por ejemplo, si la operación falla, se produce una excepción, si tiene éxito, se devuelve el empleado de actualización, etc.

Nuevamente, todo esto es cuestión de grado y juicio subjetivo.

Entonces, ¿qué pasa con la separación comando-consulta?

Bueno, ese principio existe, pero devolver el resultado de una actualización es, de hecho, muy común.

(1) Java Set#add(E)agrega el elemento y devuelve su inclusión previa en el conjunto.

if (visited.add(nodeToVisit)) {
    // perform operation once
}

Esto es más eficiente que la alternativa CQS, que puede tener que realizar dos búsquedas.

if (!visited.contains(nodeToVisit)) {
    visited.add(nodeToVisit);
    // perform operation once
}

(2) Compare-and-swap , fetch-and-add y test-and-set son primitivas comunes que permiten que exista programación concurrente. Estos patrones aparecen a menudo, desde instrucciones de CPU de bajo nivel hasta colecciones concurrentes de alto nivel.

La única responsabilidad es que una clase no necesita cambiar por más de una razón.

Como ejemplo, un empleado tiene una lista de números de teléfono. Cuando cambia la forma en que maneja los números de teléfono (puede agregar códigos de llamadas de países), eso no debería cambiar la clase de empleado en absoluto.

No quisiera que la clase de empleados necesitara saber cómo se guarda en la base de datos, porque luego cambiaría por cambios en los empleados y cambios en cómo se almacenan los datos.

Similar no debería haber un método CalculateSalary en la clase de empleado. Una propiedad salarial está bien, pero el cálculo del impuesto, etc., debe hacerse en otro lugar.

Pero que el método de actualización devuelva lo que acaba de actualizar está bien.

Wrt. El caso específico:

Employee Update(Employee, name, password, etc) (en realidad usando un Builder ya que tengo muchos parámetros).

Se parece al Updatemétodo toma una ya existenteEmployee como primer parámetro para identificar (?) El empleado existente y un conjunto de parámetros para el cambio de este empleado.

Yo no creo que esto podría ser más limpio hacerlo. Veo dos casos:

(a) en Employee realidad contiene una base de datos / ID única por la cual siempre se puede identificar en la base de datos. (Es decir, no necesita todos los valores de registro establecidos para encontrarlo en la base de datos.

En este caso, preferiría un void Update(Employee obj)método que solo encuentre el registro existente por ID y luego actualice los campos del objeto pasado. O tal vez unvoid Update(ID, EmployeeBuilder values)

Una variación de esto que encontré útil es tener solo un void Put(Employee obj)método que inserte o actualice, dependiendo de si existe el registro (por ID).

(b) Se necesita el registro existente completo para la búsqueda de DB, en ese caso aún podría tener más sentido tener:void Update(Employee existing, Employee newData) .

Por lo que puedo ver aquí, diría que la responsabilidad de construir un nuevo objeto (o valores de subobjetos) para almacenar y almacenarlo en realidad es ortogonal, por lo que los separaría.

Los requisitos concurrentes mencionados en otras respuestas (atomic set-and-retrieve / compare-swap, etc.) no han sido un problema en el código DB en el que he trabajado hasta ahora. Cuando hablo con un DB, creo que esto debería manejarse en el nivel de transacción normalmente, no en el nivel de extracto individual. (Eso no quiere decir que podría no haber un diseño en el que un "atómico" Employee[existing data] Update(ID, Employee newData)no pudiera tener sentido, pero con los accesos DB no es algo que normalmente veo).

Hasta ahora todos aquí están hablando de clases. Pero piénselo desde una perspectiva de interfaz.

Si un método de interfaz declara un tipo de retorno, y / o una promesa implícita sobre el valor de retorno, entonces cada implementación debe devolver el objeto actualizado.

Entonces puedes preguntar:

• ¿Puedes pensar en posibles implementaciones que no quieran molestarse en devolver un nuevo objeto?
• ¿Puede pensar en componentes que dependen de la interfaz (al tener una instancia inyectada), que no necesitan el empleado actualizado como valor de retorno?

También piense en los objetos simulados para las pruebas unitarias. Obviamente será más fácil burlarse sin el valor de retorno. Y los componentes dependientes son más fáciles de probar si sus dependencias inyectadas tienen interfaces más simples.

Sobre la base de estas consideraciones, puede tomar una decisión.

Y si luego te arrepientes, aún podrías introducir una segunda interfaz, con adaptadores entre los dos. Por supuesto, estas interfaces adicionales aumentan la complejidad de la composición, por lo que todo es una compensación.

Tu enfoque está bien. La inmutabilidad es una afirmación fuerte. Lo único que preguntaría es: ¿Hay algún otro lugar donde construyas el objeto? Si su objeto no era inmutable, tendría que responder preguntas adicionales porque se introduce "Estado". Y el cambio de estado de un objeto puede ocurrir por diferentes razones. Entonces debe conocer sus casos y no deben ser redundantes o divididos.