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.