www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.learn - optional feature with contract inheritance? (design question)

reply Lutger <lutger.blijdestijn gmail.com> writes:
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?
Feb 27 2010
parent reply Jonathan M Davis <jmdavisProg gmail.com> writes:
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
Feb 27 2010
parent reply Lutger <lutger.blijdestijn gmail.com> writes:
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.
Feb 27 2010
parent reply Jonathan M Davis <jmdavisProg gmail.com> writes:
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
Feb 27 2010
parent reply Lutger <lutger.blijdestijn gmail.com> writes:
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
{ 
}
Feb 27 2010
parent Jonathan M Davis <jmdavisProg gmail.com> writes:
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
Feb 27 2010