¿Es esto una violación del Principio de sustitución de Liskov?

133

Digamos que tenemos una lista de entidades de tareas y un ProjectTasksubtipo. Las tareas se pueden cerrar en cualquier momento, excepto ProjectTasksque no se pueden cerrar una vez que tienen el estado de Iniciado. La interfaz de usuario debe garantizar que la opción de cerrar un inicio ProjectTasknunca esté disponible, pero existen algunas salvaguardas en el dominio:

public class Task
{
     public Status Status { get; set; }

     public virtual void Close()
     {
         Status = Status.Closed;
     }
}

public class ProjectTask : Task
{
     public override void Close()
     {
          if (Status == Status.Started) 
              throw new Exception("Cannot close a started Project Task");

          base.Close();
     }
}

Ahora, cuando se llama Close()a una Tarea, existe la posibilidad de que la llamada falle si es ProjectTaskcon el estado iniciado, cuando no lo sería si fuera una Tarea base. Pero estos son los requisitos comerciales. Debería fallar. ¿Se puede considerar esto como una violación del principio de sustitución de Liskov ?

Paul T Davies
fuente
14
Perfecto para un ejemplo T de violar la sustitución de liskov. No uses la herencia aquí, y estarás bien.
Jimmy Hoffa
8
Es posible que desee cambiarlo a public Status Status { get; private set; }:; de lo contrario, el Close()método se puede solucionar.
Trabajo
55
Tal vez sea solo este ejemplo, pero no veo ningún beneficio material para cumplir con el LSP. Para mí, esta solución en la pregunta es más clara, más fácil de entender y más fácil de mantener que una que cumpla con LSP.
Ben Lee
2
@BenLee No es más fácil de mantener. Solo se ve de esa manera porque estás viendo esto de forma aislada. Cuando el sistema es grande, asegurarse de que los subtipos de Taskno introduzcan incompatibilidades extrañas en el código polimórfico que solo conoce Taskes un gran problema. LSP no es un capricho, pero se introdujo precisamente para ayudar a la mantenibilidad en sistemas grandes.
Andres F.
8
@BenLee Imagine que tiene un TaskCloserproceso que closesAllTasks(tasks). Este proceso obviamente no intenta atrapar excepciones; después de todo, no es parte del contrato explícito de Task.Close(). Ahora presentas ProjectTasky de repente TaskClosercomienzas a lanzar excepciones (posiblemente no manejadas). ¡Este es un gran problema!
Andres F.

Respuestas:

174

Sí, es una violación del LSP. El principio de sustitución de Liskov requiere que

  • Las condiciones previas no pueden fortalecerse en un subtipo.
  • Las condiciones posteriores no pueden debilitarse en un subtipo.
  • Las invariantes del supertipo deben conservarse en un subtipo.
  • Restricción del historial (la "regla del historial"). Los objetos se consideran modificables solo a través de sus métodos (encapsulación). Dado que los subtipos pueden introducir métodos que no están presentes en el supertipo, la introducción de estos métodos puede permitir cambios de estado en el subtipo que no están permitidos en el supertipo. La restricción de la historia lo prohíbe.

Su ejemplo rompe el primer requisito al fortalecer una condición previa para llamar al Close()método.

Puede solucionarlo llevando la precondición fortalecida al nivel superior de la jerarquía de herencia:

public class Task {
    public Status Status { get; set; }
    public virtual bool CanClose() {
        return true;
    }
    public virtual void Close() {
        Status = Status.Closed;
    }
}

Al estipular que una llamada de Close()es válida solo en el estado en el que se realizan CanClose()devoluciones, trueusted hace que la condición previa se aplique Tasktanto a la ProjectTaskreparación como a la violación del LSP:

public class ProjectTask : Task {
    public override bool CanClose() {
        return Status != Status.Started;
    }
    public override void Close() {
        if (Status == Status.Started) 
            throw new Exception("Cannot close a started Project Task");
        base.Close();
    }
}
dasblinkenlight
fuente
17
No me gusta la duplicación de ese cheque. Preferiría lanzar una excepción yendo a Task.Close y eliminar virtual de Close.
Eufórico
44
@Euphoric Eso es cierto, hacer que el nivel superior Closehaga la verificación y agregar una protección DoClosesería una alternativa válida. Sin embargo, quería estar lo más cerca posible del ejemplo del OP; mejorarlo es una pregunta separada.
dasblinkenlight
55
@Euphoric: Pero ahora no hay forma de responder la pregunta: "¿Se puede cerrar esta tarea?" sin intentar cerrarlo. Esto fuerza innecesariamente el uso de excepciones para el control de flujo. Sin embargo, admitiré que este tipo de cosas pueden llevarse demasiado lejos. Llevado demasiado lejos, este tipo de solución puede terminar produciendo un desastre empresarial. En cualquier caso, la pregunta del OP me parece más sobre principios, por lo que una respuesta de la torre de marfil es muy apropiada. +1
Brian
30
@Brian The CanClose todavía está allí. Todavía se puede llamar para verificar si la Tarea se puede cerrar. El check in Cerrar también debería llamar a esto.
Eufórico
55
@Eufórico: Ah, no entendí bien. Tienes razón, eso lo convierte en una solución mucho más limpia.
Brian
82

Si. Esto viola el LSP.

Mi sugerencia es agregar un CanClosemétodo / propiedad a la tarea base, para que cualquier tarea pueda determinar si la tarea en este estado se puede cerrar. También puede proporcionar una razón por la cual. Y eliminar el virtual de Close.

Basado en mi comentario:

public class Task {
    public Status Status { get; private set; }

    public virtual bool CanClose(out String reason) {
        reason = null;
        return true;
    }
    public void Close() {
        String reason;
        if (!CanClose(out reason))
            throw new Exception(reason);

        Status = Status.Closed;
    }
}

public ProjectTask : Task {
    public override bool CanClose(out String reason) {
        if (Status != Status.Started)
        {
            reason = "Cannot close a started Project Task";
            return false;
        }
        return base.CanClose(out reason);
    }
}
Eufórico
fuente
3
Gracias por esto, tomaste el ejemplo de dasblinkenlight una etapa más allá, pero me gustó su explicación y justificación. Lo siento, no puedo aceptar 2 respuestas!
Paul T Davies
Estoy interesado en saber por qué la firma es pública virtual bool CanClose (fuera de la razón de la cadena): al usarla, ¿estás simplemente a prueba de futuro? ¿O hay algo más sutil que me estoy perdiendo?
Reacher Gilt
3
@ReacherGilt Creo que deberías comprobar qué hace / ref hacer y leer mi código nuevamente. Estás confundido. Simplemente "Si la tarea no se puede cerrar, quiero saber por qué".
Eufórico el
2
out no está disponible en todos los idiomas, devolver una tupla (o un simple objeto que encapsula la razón y el booleano lo haría mejor portátil en los idiomas OO, aunque a costa de perder la facilidad de tener un bool directamente. Dicho esto, para los idiomas que HACEN apoyo, nada de malo con esta respuesta.
Newtopian
1
¿Y está bien fortalecer las condiciones previas para la propiedad CanClose? Es decir, agregar la condición?
John V
24

El principio de sustitución de Liskov establece que una clase base debe ser reemplazable por cualquiera de sus subclases sin alterar ninguna de las propiedades deseables del programa. Dado que solo ProjectTaskplantea una excepción cuando se cierra, un programa tendría que cambiarse para adaptarse a eso, debe ProjectTaskutilizarse en sustitución de Task. Entonces es una violación.

Pero si modifica Taskindicando en su firma que puede generar una excepción cuando se cierra, entonces no estaría violando el principio.

Tulains Córdova
fuente
Uso c #, que no creo que tenga esta posibilidad, pero sé que Java sí.
Paul T Davies
2
@PaulTDavies Puede decorar un método con las excepciones que arroja, msdn.microsoft.com/en-us/library/5ast78ax.aspx . Se da cuenta de esto cuando pasa el mouse sobre un método de la biblioteca de clases base y obtendrá una lista de excepciones. No se aplica, pero no obstante hace que la persona que llama sea consciente.
Despertar
18

Una violación de LSP requiere tres partes. El Tipo T, el Subtipo S y el programa P que usa T pero recibe una instancia de S.

Su pregunta ha proporcionado T (Tarea) y S (ProjectTask), pero no P. Por lo tanto, su pregunta está incompleta y la respuesta está calificada: si existe una P que no espera una excepción, entonces, para esa P, tiene un LSP violación. Si cada P espera una excepción, entonces no hay violación de LSP.

Sin embargo, hacer una SRP violación. El hecho de que el estado de una tarea se puede cambiar, y la política de que ciertas tareas en ciertos estados deberían no ser cambiados a otros estados, son dos responsabilidades muy diferentes.

  • Responsabilidad 1: Representar una tarea.
  • Responsabilidad 2: Implementar las políticas que cambian el estado de las tareas.

Estas dos responsabilidades cambian por diferentes razones y, por lo tanto, deben estar en clases separadas. Las tareas deben manejar el hecho de ser una tarea y los datos asociados con una tarea. TaskStatePolicy debe manejar la forma en que las tareas pasan de un estado a otro en una aplicación determinada.

Robert Martin
fuente
2
Las responsabilidades dependen en gran medida del dominio y (en este ejemplo) cuán complejos son los estados de tarea y sus modificadores. En este caso, no hay indicación de tal cosa, por lo que no hay ningún problema con SRP. En cuanto a la violación de LSP, creo que todos asumimos que la persona que llama no espera una excepción y la aplicación debe mostrar un mensaje razonable en lugar de entrar en un estado erróneo.
Eufórico
¿Unca 'Bob responde? "¡No somos dignos! ¡No somos dignos!". De todos modos ... Si cada P espera una excepción, entonces no hay violación de LSP. PERO si estipulamos que una instancia T no puede arrojar una OpenTaskException(pista, pista) y cada P espera una excepción , ¿qué dice eso sobre el código para la interfaz, no la implementación? De que estoy hablando No lo sé. Estoy emocionado porque estoy comentando una respuesta de Unca 'Bob.
radarbob
3
Tiene razón en que probar una violación de LSP requiere tres objetos. Sin embargo, la violación de LSP existe si hay CUALQUIER programa P que fue correcto en ausencia de S pero falla con la adición de S.
kevin cline
16

Esto puede o no ser una violación del LSP.

Seriamente. Escúchame.

Si sigue el LSP, los objetos de tipo ProjectTaskdeben comportarse como Taskse espera que se comporten los objetos de tipo .

El problema con su código es que no ha documentado cómo Taskse espera que se comporten los objetos de tipo . Tiene un código escrito, pero no tiene contratos. Agregaré un contrato para Task.Close. Dependiendo del contrato que agregue, el código para ProjectTask.Closeo no sigue el LSP.

Dado el siguiente contrato para Task.Close, el código para ProjectTask.Close no sigue el LSP:

     // Behaviour: Moves the task to the closed state
     // and does not throw any Exception.
     // Default behaviour: Moves the task to the closed state
     // and does not throw any Exception.
     public virtual void Close()
     {
         Status = Status.Closed;
     }

Dado el siguiente contrato para Task.Close, el código para ProjectTask.Close no seguir el camino LSP:

     // Behaviour: Moves the task to the closed status if possible.
     // If this is not possible, this method throws an Exception
     // and leaves the status unchanged.
     // Default behaviour: Moves the task to the closed state
     // and does not throw any Exception.
     public virtual void Close()
     {
         Status = Status.Closed;
     }

Los métodos que pueden anularse deben documentarse de dos maneras:

  • El "Comportamiento" documenta en qué puede confiar un cliente que sabe que el objeto receptor es un Task, pero no sabe de qué clase es una instancia directa. También le dice a los diseñadores de subclases qué anulaciones son razonables y cuáles no.

  • El "comportamiento predeterminado" documenta en qué puede confiar un cliente que sabe que el objeto receptor es una instancia directa de Task(es decir, lo que obtienes si lo usas new Task(). También le dice a los diseñadores de subclases qué comportamiento se heredará si no lo hacen anular el método

Ahora deben tener las siguientes relaciones:

  • Si S es un subtipo de T, el comportamiento documentado de S debería refinar el comportamiento documentado de T.
  • Si S es un subtipo de (o igual a) T, el comportamiento del código de S debería refinar el comportamiento documentado de T.
  • Si S es un subtipo de (o igual a) T, el comportamiento predeterminado de S debería refinar el comportamiento documentado de T.
  • El comportamiento real del código para una clase debe refinar su comportamiento predeterminado documentado.
Theodore Norvell
fuente
@ user61852 planteó el punto de que puede indicar en la firma del método que puede generar una excepción, y simplemente haciendo esto (algo que no tiene un código de efecto real) ya no está rompiendo LSP.
Paul T Davies
@PaulTDavies Tienes razón. Pero en la mayoría de los idiomas, la firma no es una buena manera de declarar que una rutina puede arrojar una excepción. Por ejemplo, en el OP (en C #, creo) la segunda implementación de Closedoes throw. Entonces, la firma declara que se puede lanzar una excepción , no dice que no se haga. Java hace un mejor trabajo en este sentido. Aun así, si declara que un método puede declarar una excepción, debe documentar las circunstancias bajo las cuales puede (o lo hará). Así que sostengo que para estar seguros de si se viola LSP, necesitamos documentación más allá de la firma.
Theodore Norvell
44
Muchas respuestas aquí parecen ignorar por completo el hecho de que no se puede saber si un contrato está validado si no se conoce el contrato. Gracias por esa respuesta
gnasher729
Buena respuesta, pero las otras respuestas también son buenas. Infieren que la clase base no arroja excepciones porque no hay nada en esa clase que muestre signos de eso. Entonces, el programa, que usa la clase base, no debe prepararse para excepciones.
inf3rno
Tiene razón en que la lista de excepciones debe documentarse en alguna parte. Creo que el mejor lugar está en el código. Aquí hay una pregunta relacionada: stackoverflow.com/questions/16700130/… Pero puede hacer esto sin anotaciones, etc. También, simplemente escriba algo parecido if (false) throw new Exception("cannot start")a la clase base. El compilador lo eliminará, y aún así el código contiene lo que se necesita. Por cierto. todavía tenemos una violación de LSP con estas soluciones, porque la condición previa todavía se fortalece ...
inf3rno
6

No es una violación del Principio de sustitución de Liskov.

El principio de sustitución de Liskov dice:

Deje que q (x) sea una propiedad comprobable sobre los objetos x de tipo T . Vamos S ser un subtipo de T . El tipo S viola el principio de sustitución de Liskov si existe un objeto y del tipo S , de modo que q (y) no sea demostrable.

La razón por la cual su implementación del subtipo no es una violación del Principio de sustitución de Liskov es bastante simple: no se puede probar nada sobre lo que Task::Close()realmente hace. Por supuesto, ProjectTask::Close()una excepción cuando Status == Status.Started, pero por lo que podría Status = Status.Closeden Task::Close().

Oswald
fuente
4

Sí, es una violación.

Te sugiero que tengas tu jerarquía al revés. Si no todos Taskse pueden cerrar, entonces close()no pertenece Task. Quizás desee una interfaz CloseableTaskque todos los que no ProjectTaskspuedan implementar.

Tom G
fuente
3
Cada tarea se puede cerrar, pero no en todas las circunstancias.
Paul T Davies
Este enfoque me parece arriesgado, ya que las personas pueden escribir código esperando que todas las Tareas implementen ClosableTask, aunque modela con precisión el problema. Estoy dividido entre este enfoque y una máquina de estado porque odio las máquinas de estado.
Jimmy Hoffa
Si Taskno se implementa, CloseableTaskentonces están haciendo un lanzamiento inseguro en algún lugar para siquiera llamar Close().
Tom G
@TomG, eso es lo que temo
Jimmy Hoffa
1
Ya hay una máquina de estado. El objeto no se puede cerrar porque está en el estado incorrecto.
Kaz
3

Además de ser un problema de LSP, parece que está usando excepciones para controlar el flujo del programa (tengo que suponer que capturas esta excepción trivial en algún lugar y haces un flujo personalizado en lugar de dejar que bloquee tu aplicación).

Parece que este es un buen lugar para implementar el patrón de estado para TaskState y dejar que los objetos de estado administren las transiciones válidas.

Ed Hastings
fuente
1

Aquí me falta algo importante relacionado con el LSP y el diseño por contrato: en las condiciones previas, es la persona que llama cuya responsabilidad es asegurarse de que se cumplan las condiciones previas. El código llamado, en teoría DbC, no debería verificar la condición previa. El contrato debe especificar cuándo se puede cerrar una tarea (por ejemplo, CanClose devuelve True) y luego el código de llamada debe garantizar que se cumpla la condición previa antes de llamar a Close ().

Ezoela Vacca
fuente
El contrato debe especificar cualquier comportamiento que el negocio necesite. En este caso, ese Close () generará una excepción cuando se inicie ProjectTask. Esta es una condición posterior (dice lo que sucede después de que se llama al método) y cumplirlo es responsabilidad del código llamado.
Goyo
@Goyo Sí, pero como otros dijeron, la excepción se plantea en el subtipo que fortaleció la condición previa y, por lo tanto, violó el contrato (implícito) de que llamar a Close () simplemente cierra la tarea.
Ezoela Vacca
¿Qué precondición? No veo ninguno
Goyo
@Goyo Verifique la respuesta aceptada, por ejemplo :) En la clase base, Cerrar no tiene condiciones previas, se llama y cierra la tarea. En el niño, sin embargo, existe una condición previa sobre el estado no iniciado. Como otros señalaron, este es un criterio más fuerte y, por lo tanto, el comportamiento no es sustituible.
Ezoela Vacca
No importa, encontré la condición previa en la pregunta. Pero entonces no hay nada de malo (DbC-wise) con el código llamado que verifica las condiciones previas y genera excepciones cuando no se cumplen. Se llama "programación defensiva". Además, si hay una condición posterior que indica lo que sucede cuando no se cumple la condición previa como en este caso, la implementación debe verificar la condición previa para garantizar que se cumpla la condición posterior.
Goyo
0

Sí, es una clara violación de LSP.

Algunas personas argumentan aquí que hacer explícito en la clase base que las subclases pueden lanzar excepciones haría esto aceptable, pero no creo que sea cierto. No importa lo que documente en la clase base o el nivel de abstracción al que mueva el código, las condiciones previas aún se fortalecerán en la subclase, porque agrega la parte "No se puede cerrar una tarea de proyecto iniciada". Esto no es algo que pueda resolver con una solución alternativa, necesita un modelo diferente, que no viole el LSP (o tenemos que aflojar la restricción "no se pueden fortalecer las condiciones previas").

Puede probar el patrón de decorador si desea evitar la violación de LSP en este caso. Podría funcionar, no lo sé.

inf3rno
fuente