Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Code Contract, inheritance and Liskov Principle

I have in my code the concept of command :

public abstract class BaseCommand
{
    public BaseCommand() { this.CommandId = Guid.NewGuid(); this.State = CommandState.Ready; }
    public Guid CommandId { get; private set; }
    public CommandState State {get; private set; }
    protected abstract void OnExecute();
    public void Execute() {
         OnExecute();
         State = CommandState.Executed;
    }
}

And some concrete implementation like this one :

public class DeleteItemCommand
{
    public int ItemId {get; set;}
    protected override void OnExecute()
    {
        var if = AnyKindOfFactory.GetItemRepository();
        if.DeleteItem(ItemId);
    }
}    

Now I want to add some validation. The first thing I can do is add a if/throw check:

public class DeleteItemCommand
{
    public int ItemId {get; set;}
    protected override void Execute()
    {
        if(ItemId == default(int)) throw new VeryBadThingHappendException("ItemId is not set, cannot delete the void");
        var if = AnyKindOfFactory.GetItemRepository();
        if.DeleteItem(ItemId);
    }
}

Now, I'm trying to use Code Contracts, because I'm quite convinced of its usefulness to reduce bug risk. If I rewrote the method like this :

public class DeleteItemCommand
{
    public int ItemId {get; set;}
    public void Execute()
    {
        Contract.Requires<VeryBadThingHappendException>(ItemId != default(int));

        var if = AnyKindOfFactory.GetItemRepository();
        if.DeleteItem(ItemId);
    }
}

The method compiles, the check is done at run-time. However, I got warning :

warning CC1032: CodeContracts: Method 'MyProject.DeleteItemCommand.Execute' overrides 'MyProject.BaseCommand.Execute', thus cannot add Requires.

I understand this warning is issued because I'm breaking the Liskov Principle.

However, in my case, conditions are different from one concrete class to another. My BaseCommand class is actually defining some common attributes like CommandIdentifier, state, and other ultimate features I removed here to keep the question simple.

While I understand the concepts of this principle, I don't know what are the step I have to do to remove the warning properly (don't tell me about the #pragma warning remove).

  1. Should I stop using code contracts in this case, where concrete implementations have specific requirements ?
  2. Should I rewrite my commanding mechanism to have, for example, separation between the Command "arguments" and the Command "execution" ? (having one CommandeExecutor<TCommand> per concrete class). This would result in a lot more classes in my project.
  3. Any other suggestion ?
  4. [Edit] As suggested by adrianm, convert properties to readonly, add constructor parameters to populate the properties and check properties in the contructor
like image 452
Steve B Avatar asked Sep 29 '11 10:09

Steve B


2 Answers

I don't think you can use code contracts in this case. I think, you cannot add preconditions in overridden methods, only invariants and postconditions are possible there. Your best bet might be be to refactor from inheritance to composition:

ICommandExecutor
{
    Execute(BaseCommand source);
}

public abstract class BaseCommand
{
    public ICommandExecutor Executor { get; private set; }
    public void Execute() 
    {
        this.Executor.Execute(this);
        State = CommandState.Executed;
    }
}

public class DeleteCommandExecutor : ICommandExecutor
{
    public void Execute(BaseCommand source)
    {
        Contract.Requires<VeryBadThingHappendException>(source.ItemId != default(int));
        var if = AnyKindOfFactory.GetItemRepository();
        if.DeleteItem(source.ItemId);
    }
}
like image 151
bitbonk Avatar answered Sep 28 '22 07:09

bitbonk


You could change the code to perform the execution in a different method:

public class DeleteItemCommand: BaseCommand
{
    public int ItemId {get; set;}
    public override void Execute()
    {
        PrivateExecute(ItemId);
    }

    private void PrivateExecute(int itemId)
    {
        Contract.Requires<VeryBadThingHappendException>(itemId != default(int));

        var rep = AnyKindOfFactory.GetItemRepository();
        rep.DeleteItem(itemId);
    }
}
like image 44
Hans Kesting Avatar answered Sep 28 '22 09:09

Hans Kesting