View mode: basic / threaded / horizontal-split · Log in · Help
February 27, 2010
optional feature with contract inheritance? (design question)
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
Re: optional feature with contract inheritance? (design question)
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
Re: optional feature with contract inheritance? (design question)
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
Re: optional feature with contract inheritance? (design question)
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
Re: optional feature with contract inheritance? (design question)
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
Re: optional feature with contract inheritance? (design question)
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
Top | Discussion index | About this forum | D home