Codex contract, inheritance and Liskov principle

My code has the concept of a 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 specific implementations 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 an 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 am trying to use Code Contracts, because I am quite confident in its usefulness to reduce the risk of errors. If I rewrote the method as follows:

 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 is compiled, verification is performed at runtime. However, I received a warning:

warning CC1032: CodeContracts: the method "MyProject.DeleteItemCommand.Execute" overrides "MyProject.BaseCommand.Execute", so it cannot add Requires.

I understand that this warning is issued because I violate the Liskov principle.

However, in my case, the conditions differ from one particular class to another. My BaseCommand class actually defines some common attributes, such as CommandIdentifier, state, and other final functions, which I removed here to make a simple question.

As long as I understand the concepts of this principle, I don’t know what I need to do to properly remove the warning (do not tell me about #pragma warning remove ).

  • Should I stop using code contracts in this case, where specific implementations have special requirements?
  • Do I have to rewrite my command mechanism to have, for example, a separation between the “arguments” and “execution” commands? (having one CommandeExecutor<TCommand> for a particular class). This will lead to a significant increase in the number of classes in my project.
  • Any other suggestion?
  • [Change] As suggested by adrianm , convert properties to readonly, add constructor parameters to populate properties, and check properties in constructor
+7
source share
5 answers

I would use a constructor to set all the correct values, not public property settings.

 public class DeleteItemCommand { public DeleteItemCommand(int itemId) { Contract.Requires<VeryBadThingHappendException>(itemId!= default(int)); ItemId = itemId; } public int ItemId {get; private set;} public void Execute() { var if = AnyKindOfFactory.GetItemRepository(); if.DeleteItem(ItemId); } } 
+1
source

I do not think you can use code contracts in this case. I think you cannot add preconditions to overridden methods, only invariants and postconditions are possible there. It’s best 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); } } 
+5
source

You can change the code to execute another 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); } } 
+2
source

In this case, code contracts direct your attention to a design error in your class structure, and it would be wise for you to heed this warning.

To see the problem, consider how classes can be used.

 protected void executeButton_Click(object sender, EventArgs args) { BaseCommand command = GetCurrenctlySelectedCommand(); command.Execute(); } 

If the command variable has an object of type DeleteItemCommand , then this object has preconditions that must be met or an exception will be thrown. We would like to avoid this exception, so how can we make sure that the condition is met?

Unfortunately, there is no easy way to do this. We cannot talk about all the possible assumptions about each type of derivative object that can live in this variable. In fact, this variable may contain an object type that was not invented when writing this code. In fact, the type for this object may not even be in the scope of this method if it was provided by the factory in another assembly.

Since it is not possible to verify that the prerequisites for this object are met, we cannot guarantee the correctness of this code. We can conclude from this that code contracts are useless or that code is not designed correctly.

I understand that this warning is issued because I violate the Liskov principle.

So you acknowledge it!

However, in my case, the conditions are different from one particular class to another. My BaseCommand class actually defines some common attributes, such as CommandIdentifier, state, and other final functions, which I removed here to make a simple question.

Your analysis alone offers a suitable alternative.

Instead of the abstract BaseCommand class BaseCommand you must create a specific private CommandAttributes class. Then include an instance of this class in all of your command objects.

Using composition rather than inheritance, each of your command classes gets the functionality they need, and they can define any types of preconditions or postconditions that they need. And any methods that use these classes can test these preconditions and take advantage of postconditions.

+2
source

Code contracts seem to work best on domain objects. I assume that your Command class is not a domain object.

0
source

All Articles