Code Contract Best Practices

Question

I have several questions regarding code contracts, and the best practices for their usage. Lets say we have a class, with several properties (see below for example):

class Class1
{
    // Fields
    private string _property1;       //Required for usage
    private List<object> _property2; //Not required for usage

    // Properties
    public string Property1 
    { 
        get
        {
            return this._property1;
        }            
        set
        {
            Contract.Requires(value != null);
            this._property1 = value;
        } 
    }
    public List<object> Property2 
    { 
        get
        {
            return this._property2;
        }
        set
        {
            Contract.Requires(value != null);
            this._property2 = value;
        }
    }

    public Class1(string property1, List<object> property2)
    {
        Contract.Requires(property1 != null);
        Contract.Requires(property2 != null);

        this.Property1 = property1;
        this.Property2 = property2;
    }

    public Class1(string property1)
        : this(property1, new List<object>())
    { }
}

Some explanation about what I want to achieve:

(a) property1 is a required field. property2 is not explicitly required for normal usage of the object.

I have the following questions:

1. Should I even bother with the contracts for property2; because property2 is not a required field, should it have a contract at all. Does placing a contract on property2 indicate that it is in fact required for normal usage of the object;

2. Even though property2 is not explicitly required, there is no possible reason for it to be null, thus the defined contract at the setter. Wouldn't defining the contract on property2 reduce the null checks in calling code? This should reduce bugs and improve maintainability of the code - is this assumption correct?

3. If it is right, how do I ensure to calling code that property2 will never be null? Do I use Contract.Invariant(property2 != null); or Contract.Ensures(property2 != null) in the constructor, or Contract.Ensures(property2 != null) in the Init(), or Contract.Ensures(property != null) in the setter? (i.e. if using Contract.Ensures(property2 != null), where is it placed)?

My apologies if the questions seem simple. I am just looking for thoughts on the matter, and what you folks consider best practice.

Answer 1

This is what I would recommend as far as Contracts:

    class Class1
    {
        // Fields
        private string _property1;       //Required for usage
        private List<object> _property2; //Not required for usage

        // Properties
        public string Property1
        {
            get
            {
                Contract.Ensures(Contract.Result<string>() != null);
                return this._property1;
            }
            set
            {
                Contract.Requires(value != null);
                this._property1 = value;
            }
        }

        public List<object> Property2
        {
            get
            {
                Contract.Ensures(Contract.Result<List<object>>() != null);
                return this._property2;
            }
            set
            {
                Contract.Requires(value != null);
                this._property2 = value;
            }
        }

        public Class1(string property1, List<object> property2)
        {
            Contract.Requires(property1 != null);
            Contract.Requires(property2 != null);

            this.Property1 = property1;
            this.Property2 = property2;
        }

        public Class1(string property1)
            : this(property1, new List<object>())
        {
            Contract.Requires(property1 != null);
        }

        [ContractInvariantMethod]
        private void ContractInvariants()
        {
            Contract.Invariant(_property1 != null);
            Contract.Invariant(_property2 != null);
        }
    }

The properties have their public behavioral contracts and the Invariants will catch any errors you might introduce later as you add logic to Class1 that could modify the field values and thus violate the public contracts. Alternately, if the fields can be made readonly (and the setters removed), you don't need the invariants.

Answer 2

I think there's a lot of personal preference in here, but my 2 cents...

1) I would, and would possibly be tempted to adjust the constructors to have property2 as an optional argument:

Class1(string property1, List<object> property2 = new List<object>())      
{      
    Contract.Requires(property1 != null);      
    Contract.Requires(property2 != null);      

    this.Property1 = property1;      
    this.Property2 = property2;      
} 

2) See 3

3) Before I'd be happy to reduce null checks in calling code, I would personally prefer to see a

Contract.Ensures(property2 != null) 

on the getter - I've seen VS11 CTP shows contracts in the tooltips for definitions so being able to see this I would then know I don't need to check for nulls. I would keep the Requires on the setter.

Answer 3

Often when you have a List in an object you will have the object take care of the creation. So if a consumer wishes to add to a list they must first get it. Depending on the usage of this class this might be the better route.

class Class1
{
    // Fields
    private string _property1;       //Required for usage
    private List<object> _property2 = new List<object>(); //Not required for usage

    // Properties
    public string Property1 
    { 
        get
        {
            return this._property1;
        }            
        set
        {
            Contract.Requires(value != null);
            this._property1 = value;
        } 
    }
    public List<object> Property2 
    { 
        get
        {
            return this._property2;
        }
        //We don't have a setter.
    }

    public Class1(string property1)
    {
        Contract.Requires(property1 != null);

        this.Property1 = property1;
    }
}

Answer 4

1/2: Having a contract on property2 is appropriate if that is the behaviour you want to enforce i.e. It should not be null, and will certainly reduce the need for null checks and potential bugs around nulls.

3: To answer this question I have rewritten your class as follows

class Class1
{
    //Properties
    public string Property1 { get; set; }
    public List<object> Property2 { get; set; }

    public Class1(string property1, List<object> property2)
    {
        Contract.Requires(property1 != null);
        Contract.Requires(property2 != null);

        Property1 = property1;
        Property2 = property2;
    }

    public Class1(string property1)
        : this(property1, new List<object>())
    { 
        Contract.Requires(property1 != null);
    }

    [ContractInvariantMethod]
    private void ObjectInvariant()
    {
        Contract.Invariant(Property1 != null);
        Contract.Invariant(Property2 != null);
    }
}

When you have auto implemented properties in your class you can use Contract.Invariant(). This makes explicit contracts in your property redundant so there is no need for the following code now.

public string Property1 
{ 
    get
    {
        Contract.Ensures(Contract.Result<string>() != null);
        return this._property1;
    }            
    set
    {
        Contract.Requires(value != null);
        this._property1 = value;
    } 
}

That will take care of the property protection. To fully ensure that the property will never be null you will then add a Contract.Requires(property1 != null) in the constructor.

I know this answer is 3 years late but it may be of some use to you!

Source: http://research.microsoft.com/en-us/projects/contracts/userdoc.pdf