¿Es aplicable el principio de responsabilidad única a las funciones?

17

Según Robert C. Martin, el SRP afirma que:

Nunca debe haber más de una razón para que una clase cambie.

Sin embargo, en su libro Clean Code , capítulo 3: Funciones, muestra el siguiente bloque de código:

    public Money calculatePay(Employee e) throws InvalidEmployeeType {
        switch (e.type) {
            case COMMISSIONED:
                return calculateCommissionedPay(e);
            case HOURLY:
                return calculateHourlyPay(e);
            case SALARIED:
                return calculateSalariedPay(e);
            default:
                throw new InvalidEmployeeType(e.type);
        }
    }

Y luego dice:

Hay varios problemas con esta función. Primero, es grande, y cuando se agreguen nuevos tipos de empleados, crecerá. Segundo, claramente hace más de una cosa. Tercero, viola el Principio de Responsabilidad Única (SRP) porque hay más de una razón para que cambie . [énfasis mío]

En primer lugar, pensé que el SRP estaba definido para las clases, pero resulta que también es aplicable a las funciones. En segundo lugar, ¿cómo es que esta función tiene más de una razón para cambiar ? Solo puedo verlo cambiar debido a un cambio en Empleado.

Enrique
fuente
55
Esto parece un caso de libro de texto para el polimorfismo.
wchargin
Este es un tema muy interesante. ¿Existe la posibilidad de que agregue la siguiente solución a este problema? Seguramente uno colocaría una función de cálculo de pago en cada clase de empleado, pero eso sería un mal porque ahora cada clase de empleado se puede cambiar debido a: 1. Los cálculos de pago. 2. Agregar más propiedades a la clase, etc.
Stav Alfi

Respuestas:

13

Un detalle que a menudo se pierde del Principio de Responsabilidad Única es que las "razones para el cambio" están agrupadas por actores de casos de uso (puede ver una explicación completa aquí ).

Entonces, en su ejemplo, el calculatePaymétodo deberá cambiarse siempre que se requieran nuevos tipos de Empleados. Dado que un tipo de empleado puede no tener nada que ver con otro, sería una violación del principio si los mantiene juntos, ya que el cambio afectaría a diferentes grupos de usuarios (o actores de casos de uso) en el sistema.

Ahora, sobre si el principio se aplica a las funciones: incluso si tiene una violación en un solo método, todavía está cambiando una clase por más de una razón, por lo que sigue siendo una violación de SRP.

MichelHenrich
fuente
1
Traté de ver el video de YouTube vinculado, pero después de 10 minutos sin mencionar la agrupación por actores de casos de uso, me di por vencido. Los primeros 6 minutos están divagando sobre la entropía, sin razón aparente. Si le dio una ubicación en el video donde comienza a discutir esto, sería útil.
Michael Shaw
@MichaelShaw Intenta mirar desde las 10:40 en adelante. El tío Bob menciona que el código "cambiará por diferentes razones, debido a diferentes personas". Creo que eso podría ser lo que MichelHenrich intenta señalarnos.
Enrique
Terminé de ver el video completo de YouTube de 50 minutos, la mayoría de los cuales no se trataba de lo que se suponía que debía aclarar. Me di cuenta en la marca de las 16:00 que ya había pasado de ese tema, y ​​nunca volvió a él. La "explicación" esencialmente se reduce a esto: "responsabilidad" en SRP no significa eso, significa "diferentes razones para el cambio", lo que realmente significa "cambios a petición de diferentes personas", lo que realmente significa "cambios en el momento solicitud de diferentes roles que desempeñan las personas ". La "explicación" no aclara nada, reemplaza una palabra vaga por otra.
Michael Shaw
2
@MichaelShaw, como en la cita del libro, si necesita introducir diferentes tipos de empleados, debe cambiar la clase de Empleado. Los diferentes roles pueden ser responsables del pago de diferentes tipos de empleados, por lo que en este caso, la clase de Empleado debe cambiarse para más de un rol, de ahí la violación de SRP.
MichelHenrich
1
@MichaelShaw sí, tienes razón: SRP depende de cómo esté organizada la organización. Eso es exactamente por qué agrego "may" o "might" a todos mis comentarios :). Sin embargo, incluso en esos casos, si bien el código puede no violar SRP, seguramente viola OCP.
MichelHenrich
3

En la página 176, Capítulo 12: Emergencia, en la sección titulada Clases y métodos mínimos, el libro proporciona una especie de corrección al afirmar:

En un esfuerzo por hacer que nuestras clases y métodos sean pequeños, podríamos crear demasiadas clases y métodos pequeños. Entonces, esta regla sugiere que también mantengamos bajas nuestras funciones y recuentos de clases

y

Los recuentos de clase y método altos a veces son el resultado de un dogmatismo sin sentido.

Obviamente, él está hablando del dogmatismo al seguir el SRP para desglosar pequeños métodos inocentes perfectamente bien como los calculatePay()anteriores.

Mike Nakis
fuente
3

Cuando el Sr. Martin aplica el SRP a una función, está ampliando implícitamente su definición de SRP. Como el SRP es una redacción específica de OO de un principio general, y dado que es una buena idea cuando se aplica a las funciones, no veo ningún problema con eso (aunque podría haber sido bueno si lo hubiera incluido explícitamente en el definición).

Tampoco veo más de una razón para cambiar, y no creo que sea útil pensar en el SRP en términos de "responsabilidades" o "razones para cambiar". Esencialmente, a lo que se refiere el SRP es que las entidades de software (funciones, clases, etc.) deberían hacer una cosa y hacerlo bien.

Si echas un vistazo a mi definición, no es menos vago que la redacción habitual del SRP. El problema con las definiciones habituales del SRP no es que sean demasiado vagas, sino que intentan ser demasiado específicas sobre algo que es esencialmente vago.

Si observa lo que calculatePayhace, claramente está haciendo una sola cosa: despacho basado en el tipo. Dado que Java tiene formas integradas de hacer despachos basados ​​en tipos, calculatePayes poco elegante y no idiomático, por lo que debe reescribirse, pero no por las razones indicadas.

Michael Shaw
fuente
-2

Tienes razón @Enrique. No importa si es una función o un método de una clase, el SRP significa que en ese bloque de código solo hace una cosa.

La declaración 'razón para el cambio' a veces es un poco engañoso, pero si cambia calculateSalariedPayo calculateHourlyPayo la enumeración de Employee.typeusted tiene que cambiar este método.

En su ejemplo, la función:

  • comprueba el tipo de empleado
  • llama a otra función que calcula el dinero según el tipo

En mi opinión, no es directamente una violación de SRP, ya que los casos de cambio y las llamadas no pueden escribirse más cortas, si piensa en Employee y los métodos ya existen. De todos modos, es una violación clara del principio abierto-cerrado (OCP), ya que debe agregar declaraciones de 'caso' si agrega tipos de empleados, por lo que es una mala implementación: refactorícela.

No sabemos cómo se debe calcular el 'Dinero', pero la forma más fácil es tener Employeecomo interfaz y algunas implementaciones concretas con getMoneymétodos. En ese caso, toda la función es innecesaria.

Si es más complicado calcularlo, uno podría usar el patrón de visitante que tampoco es 100% SRP pero es más OCP que una caja de interruptor.

Aitch
fuente
2
No estoy seguro de cómo puede enumerar 2 cosas que hace la función, pero dice que no es una violación de SRP.
JeffO
@JeffO: Eso no son 2 cosas, son 2 partes de una cosa: llamar a la función apropiada según el tipo.
Michael Shaw