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.

Locker

I agree with the contract on property 2 for the reasons you listed. – Leipzig

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.

Speciosity

I would agree on the ce's in the setters. After reading the documentation in the code contract samples, it would appear that this is how the developers intended it to be. And yes to the invariants catching violations later on. Thank you, you really aided my understanding of this. – Locker

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.

Merideth

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

Irreverent

Thanks for the info on the list aspect. It was one of my concerns, that is why I included it as my non-required property. I was primarily looking for advice on how to handle the contracts for a "non-essential" property, regardless of whether it is a list or not. Any comments? – Locker

In the case where it is not a Collection i would follow the advice of Smudge. By creating a default in the Constructor and Requiring it to be not null, it tells the consumer of the API that IF they supply one, they should supply a non-null value. – Irreverent

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

Urumchi