Thread overview
optional feature with contract inheritance? (design question)
Feb 27, 2010
Lutger
Feb 27, 2010
Jonathan M Davis
Feb 27, 2010
Lutger
Feb 27, 2010
Jonathan M Davis
Feb 27, 2010
Lutger
Feb 28, 2010
Jonathan M Davis
February 27, 2010
What do you think of the following use of contracts?

class Base
{
    void useFeatureA()
    in { assert(false, "Base does not implement feature A"); }
    body
    {
        /* throw here? */
    }

    bool hasFeatureA()
    {
        return false;
    }
}

class Derived : Base
{
    override void useFeatureA()
    in { assert(true); }
    body
    {
        /* do feature A */
    }

    override bool hasFeatureA()
    {
        return true;
    }
}

In .NET this is supposedly called the optional feature pattern, recommended to use if you want to prevent factoring into too many interfaces. In .NET A.useFeatureA would just throw of course.

Adding preconditions to functions in a derived class loosens the contract, or adds to the set of acceptable inputs so to speak. In this case, the polymorphic type of the object is considered as the (sole) input.

Does it make sense? Is this a good/bad/ugly way of using contracts?
February 27, 2010
Lutger wrote:

> What do you think of the following use of contracts?
> 
> class Base
> {
>     void useFeatureA()
>     in { assert(false, "Base does not implement feature A"); }
>     body
>     {
>         /* throw here? */
>     }
> 
>     bool hasFeatureA()
>     {
>         return false;
>     }
> }
> 
> class Derived : Base
> {
>     override void useFeatureA()
>     in { assert(true); }
>     body
>     {
>         /* do feature A */
>     }
> 
>     override bool hasFeatureA()
>     {
>         return true;
>     }
> }
> 
> In .NET this is supposedly called the optional feature pattern, recommended to use if you want to prevent factoring into too many interfaces. In .NET A.useFeatureA would just throw of course.
> 
> Adding preconditions to functions in a derived class loosens the contract, or adds to the set of acceptable inputs so to speak. In this case, the polymorphic type of the object is considered as the (sole) input.
> 
> Does it make sense? Is this a good/bad/ugly way of using contracts?

Actually, I don't think that there's much point to the contracts here. assert(true) in the second one is pointless. It will always pass. And assert(false ...) in the second one doesn't really add anything either. It'll throw an AssertError when the function itself would have thrown an exception anyway (albeit presumably of a different type).

Generally, I speaking, I think that having functions which override functions from a base class and throw if they're used is a bad design. It may be that it makes perfect sense in some contexts, but generally, I'd consider it a very bad idea. Personally, I hate it when APIs do that.

Regardless of whether you consider that a good or bad idea though, there's no point in adding contracts into it. They don't add anything. If the feature is supported by the class, then no exception gets thrown. If it isn't, then an exception gets thrown. All adding asserts in preconditions does is make it an AssertError instead of whatever kind of exception you're throwing in the body. It doesn't add anything.

- Jonathan M Davis
February 27, 2010
Jonathan M Davis wrote:
...
> 
> Generally, I speaking, I think that having functions which override functions from a base class and throw if they're used is a bad design. It may be that it makes perfect sense in some contexts, but generally, I'd consider it a very bad idea. Personally, I hate it when APIs do that.

I agree, but it's sometimes hard to avoid. For example if you want a
composite and component class to have a common interface, some operations
won't make sense for components. The alternative to throwing is a no-op or
return a null object. Or asserting.

> Regardless of whether you consider that a good or bad idea though, there's no point in adding contracts into it. They don't add anything. If the feature is supported by the class, then no exception gets thrown. If it isn't, then an exception gets thrown. All adding asserts in preconditions does is make it an AssertError instead of whatever kind of exception you're throwing in the body. It doesn't add anything.
> 
> - Jonathan M Davis

True, it only adds AssertError and that could be replaced with regular asserts.

Thanks.











February 27, 2010
Lutger wrote:

> 
> True, it only adds AssertError and that could be replaced with regular asserts.
> 
> Thanks.

??? Regular asserts? When asserts in D fail, they throw an exception of type AssertError. So, unless you're talking about explicitly throwing an AssertError yourself (which seems rather silly to me), I don't know what you could mean by the difference of having an AssertError get thrown from having a "regular" assert.

- Jonathan M Davis
February 27, 2010
Jonathan M Davis wrote:

> Lutger wrote:
> 
>> 
>> True, it only adds AssertError and that could be replaced with regular asserts.
>> 
>> Thanks.
> 
> ??? Regular asserts? When asserts in D fail, they throw an exception of type AssertError. So, unless you're talking about explicitly throwing an AssertError yourself (which seems rather silly to me), I don't know what you could mean by the difference of having an AssertError get thrown from having a "regular" assert.
> 
> - Jonathan M Davis

I wasn't clear, this is what I meant with 'regular' asserts:

void foo()
{
    assert(precondition);
}

instead of:

void foo()
in
{
    assert(precondition);
}
body
{
}



February 28, 2010
Lutger wrote:

> Jonathan M Davis wrote:
> 
>> Lutger wrote:
>> 
>>> 
>>> True, it only adds AssertError and that could be replaced with regular asserts.
>>> 
>>> Thanks.
>> 
>> ??? Regular asserts? When asserts in D fail, they throw an exception of type AssertError. So, unless you're talking about explicitly throwing an AssertError yourself (which seems rather silly to me), I don't know what you could mean by the difference of having an AssertError get thrown from having a "regular" assert.
>> 
>> - Jonathan M Davis
> 
> I wasn't clear, this is what I meant with 'regular' asserts:
> 
> void foo()
> {
>     assert(precondition);
> }
> 
> instead of:
> 
> void foo()
> in
> {
>     assert(precondition);
> }
> body
> {
> }

Ah, okay. Normally, both would be there in debug mode and neither would be there in release mode, though I believe that if you were to use the unittest flag in release mode, you'd get the asserts inside functions but not the ones in contracts. So, there _could_ be a difference, but in most cases, there wouldn't be. Naturally, if you really want to make it so that calling a particular function throws an exception, you're going to want to throw that exception yourself rather than using an assertion.

- Jonathan M Davis