Implementation of non-void interface Code Contracts - default(T) vs throw NotImplementedException

Question

This may be mostly a question of style but when defining code contracts for non-void interface members, which approach is best:

Interface:

[ContractClass(typeof(IFooContract))]
public interface IFoo
{
    object Bar();
}

Contract option 1:

[ContractClassFor(typeof(IFoo))]
public abstract class IFooContract : IFoo
{
    object IFoo.Bar()
    {
        Contract.Ensures(Contract.Result<object>() != null);
        throw new NotImplementedException();
    }
}

Contract option 2:

[ContractClassFor(typeof(IFoo))]
public abstract class IFooContract : IFoo
{
    object IFoo.Bar()
    {
        Contract.Ensures(Contract.Result<object>() != null);
        return default(T);
    }
}

Most of the literature I've seen tend towards option 2 but I feel that option 1 is better as it is clearer that this is purely about the contract (and option 2 is technically breaking the contract it just defined).

Are there any scenarios where option 2 is preferred over option 1?

stakx - no longer contributing · Accepted Answer · 2014-03-18T20:05:33.397

2

Throwing an exception is semantically more correct, because the contract class can no longer be invoked and used in a seemingly reasonable way; callers will be stopped and told about their error.

However, NotImplementedException does not seem to be the proper exception to throw: That exception usually marks a section of code (such as a method body) that has yet to be implemented. But a non-void contract method has been implemented; it just isn't meant to be called. Thus I would prefer an InvalidOperationException or a NotSupportedException.

(You could even throw a custom exception type, e.g. a NotMeantToBeCalledException.)

edited Mar 18 '14 at 20:05

answered Mar 18 '14 at 07:29

stakx - no longer contributing

83,039
20
168
268

I'm guessing the `NotImplementedException` stems from the default behaviour of right clicking on an interface and selecting `implement interface` – StuartLC Mar 18 '14 at 07:41
@StuartLC: That is possible. But since the method bodies are going to be edited anyway (i.e. the addition of `Contract.Ensures`, `Contract.Requires`, etc.), why not edit a little more and change the type of exception thrown. – stakx - no longer contributing Mar 18 '14 at 08:13
I agree. I think InvalidOperationException is the way to go. – Eamon Jul 09 '14 at 02:57

StuartLC · Answer 2 · 2014-03-18T08:31:05.933

This should be an entirely subjective (and academic) argument, given that the code in the ContractClassFor class should never be called, ever. :-). As you've hinted (via non-void), the exception or default route is only required for the class to compile and isn't relevant to the contracts contained in the methods.

Obviously the abstract class cannot be directly instantiated, however, you should reduce the visibility of the ContractClassFor class to internal to reduce the changes of it being accidentally subclassed.

Additionally, Jon Skeet goes further and adds a private parameterless constructor to prevent any chance of instantiation:

private IFooContract() { }

(And to answer your question indirectly, by further expressing the fact that this is a "special" contracts helper class working around the fact that the contracts can't be placed in the interface directly, a reader of your code will easily recognise it as such just ignore the default / exception compiler placebo)

+1 for hinting at a private parameter-less constructor. – stakx - no longer contributing Mar 18 '14 at 21:21 — stakx - no longer contributing, Mar 18 '14 at 21:21

Implementation of non-void interface Code Contracts - default(T) vs throw NotImplementedException

2 Answers2

Linked