Si tratta di una violazione del principio di sostituzione di Liskov?

Sì, è una violazione dell'LSP. Il principio di sostituzione di Liskov richiede che

• Le precondizioni non possono essere rafforzate in un sottotipo.
• Le postcondizioni non possono essere indebolite in un sottotipo.
• Gli invarianti del supertipo devono essere conservati in un sottotipo.
• Limitazione cronologia (la "regola della cronologia"). Gli oggetti sono considerati modificabili solo attraverso i loro metodi (incapsulamento). Poiché i sottotipi possono introdurre metodi che non sono presenti nel supertipo, l'introduzione di questi metodi può consentire modifiche di stato nel sottotipo che non sono consentite nel supertipo. Il vincolo della cronologia proibisce questo.

Il tuo esempio rompe il primo requisito rafforzando una precondizione per chiamare il metodo Close() .

Puoi correggerlo portando la pre-condizione rafforzata al livello più alto della gerarchia dell'ereditarietà:

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

Stabilendo che una chiamata di Close() è valida solo nello stato quando CanClose() restituisce true , fai applicare la pre-condizione a Task e a ProjectTask , correggendo l'LSP violazione:

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();
    }
}

Sì. Questo viola LSP.

Il mio suggerimento è di aggiungere CanClose metodo / proprietà all'attività di base, così ogni attività può dire se l'attività in questo stato può essere chiusa. Può anche fornire un motivo per cui. E rimuovi il virtuale da Close .

In base al mio commento:

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);
    }
}

Il principio di sostituzione di Liskov afferma che una classe base dovrebbe essere sostituibile con una qualsiasi delle sue sottoclassi senza alterare nessuna delle proprietà desiderabili del programma. Poiché solo ProjectTask genera un'eccezione quando è chiusa, un programma dovrebbe essere modificato in acommodate per quello, dovrebbe essere utilizzato ProjectTask in sostituzione di Task . Quindi è una violazione.

Ma se modifichi Task affermando nella sua firma che potrebbe sollevare un'eccezione quando è chiusa, non violerai il principio.

Una violazione LSP richiede tre parti. Il tipo T, il sottotipo S e il programma P che utilizza T, ma viene data un'istanza di S.

La tua domanda ha fornito T (Task) e S (ProjectTask), ma non P. Quindi la tua domanda è incompleta e la risposta è qualificata: Se esiste una P che non prevede un'eccezione, per quella P, tu avere una violazione LSP. Se ogni P si aspetta un'eccezione, non vi è alcuna violazione LSP.

Tuttavia, fai hai una violazione SRP . Il fatto che lo stato di un'attività possa essere modificato e la politica che certe attività in determinati stati dovrebbero non essere cambiate in altri stati, sono due responsabilità molto diverse.

• Responsabilità 1: rappresenta un'attività.
• Responsabilità 2: implementa le politiche che cambiano lo stato delle attività.

Queste due responsabilità cambiano per ragioni diverse e pertanto dovrebbero essere in classi separate. Le attività devono gestire il fatto di essere un'attività e i dati associati a un'attività. TaskStatePolicy deve gestire il passaggio delle attività da stato a stato in una determinata applicazione.

Questo potrebbe o meno essere una violazione dell'LSP.

Scherzi a parte. Ascoltami.

Se segui l'LSP, gli oggetti di tipo ProjectTask devono comportarsi come si comportano gli oggetti di tipo Task .

Il problema con il tuo codice è che non hai documentato come si comportano gli oggetti di tipo Task . Hai scritto codice, ma non contratti. Aggiungerò un contratto per Task.Close . A seconda del contratto che aggiungo, il codice per ProjectTask.Close fa o non segue l'LSP.

Dato il seguente contratto per Task.Close, il codice per ProjectTask.Close non segue l'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;
     }

Dato il seguente contratto per Task.Close, il codice per ProjectTask.Close fa segue l'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;
     }

I metodi che possono essere sovrascritti devono essere documentati in due modi:

• Il "comportamento" documenta ciò che può essere invocato da un client che sa che l'oggetto destinatario è un Task , ma non sa quale classe sia un'istanza diretta di. Indica anche ai progettisti di sottoclassi le cui sostituzioni sono ragionevoli e che non sono ragionevoli.

• Il "comportamento predefinito" documenta ciò che può essere invocato da un client che sa che l'oggetto destinatario è un'istanza diretta di Task (cioè ciò che ottieni se usi new Task() . di sottoclassi quale comportamento verrà ereditato se non sovrascrive il metodo.

Ora le seguenti relazioni dovrebbero contenere:

• Se S è un sottotipo di T, il comportamento documentato di S dovrebbe affinare il comportamento documentato di T.
• Se S è un sottotipo di (o uguale a) T, il comportamento del codice di S dovrebbe affinare il comportamento documentato di T.
• Se S è un sottotipo di (o uguale a) T, il comportamento predefinito di S dovrebbe affinare il comportamento documentato di T.
• Il comportamento effettivo del codice per una classe dovrebbe perfezionare il suo comportamento predefinito documentato.

Non è una violazione del Principio di sostituzione di Liskov.

Il principio di sostituzione di Liskov dice:

Let q(x) be a property provable about objects x of type T. Let S be a subtype of T. Type S violates the Liskov Substitution Principle if an object y of type S exists, such that q(y) is not provable.

La ragione, perché la tua implementazione del sottotipo non è una violazione del Principio di sostituzione di Liskov, è piuttosto semplice: non si può dimostrare nulla su cosa faccia effettivamente Task::Close() . Certo, ProjectTask::Close() genera un'eccezione quando Status == Status.Started , ma potrebbe anche Status = Status.Closed in Task::Close() .

Sì, è una violazione.

Suggerirei di avere la tua gerarchia al contrario. Se non ogni Task è chiudibile, close() non appartiene a Task . Forse vuoi un'interfaccia, CloseableTask che tutta la% nonProjectTasks possa implementare.

Oltre ad essere un problema di LSP, sembra che stia usando le eccezioni per controllare il flusso del programma (devo presumere che si rilevi questa banale eccezione da qualche parte e si faccia un po 'di flusso personalizzato piuttosto che lasciarlo in crash).

Sembra che sia un buon posto per implementare il pattern State per TaskState e lasciare che gli oggetti di stato gestiscano le transizioni valide.

Qui mi manca una cosa importante relativa a LSP e Design by Contract: nelle precondizioni, è il chiamante la cui responsabilità è assicurarsi che le condizioni preliminari siano soddisfatte. Il codice chiamato, nella teoria DbC, non dovrebbe verificare la condizione preliminare. Il contratto deve specificare quando un'attività può essere chiusa (ad esempio CanClose restituisce True) e quindi il codice chiamante deve garantire che la condizione sia soddisfatta, prima che venga chiamato Close ().

Sì, è una chiara violazione di LSP.

Alcuni sostengono qui che rendere esplicito nella classe base che le sottoclassi possono generare eccezioni lo renderebbe accettabile, ma non penso che sia vero. A prescindere da ciò che si documenta nella classe base o in quale livello di astrazione viene spostato il codice, le condizioni preliminari verranno comunque rafforzate nella sottoclasse, poiché si aggiungerà la parte "Impossibile chiudere una parte di attività avviata". Questo non è qualcosa che puoi risolvere con una soluzione alternativa, hai bisogno di un modello diverso, che non viola LSP (o dobbiamo allentare sul vincolo "precondizioni non può essere rafforzato").

Puoi provare il motivo decoratore se vuoi evitare la violazione LSP in questo caso. Potrebbe funzionare, non lo so.