dolfin team mailing list archive

[Anders Logg <logg@xxxxxxxxx>]

I think we should try to get rid of default arguments in all virtual
functions. I have started looking at removing as much as I can.

--
Anders


On Mon, Sep 07, 2009 at 12:03:51PM +0200, Johan Hake wrote:
> On Monday 07 September 2009 10:01:26 Garth N. Wells wrote:
> > Ola Skavhaug wrote:
> > > On Mon, Sep 7, 2009 at 9:21 AM, Ola Skavhaug<skavhaug@xxxxxxxxx> wrote:
> > >> We've run into problems since only GenericMatrix provides the default
> > >> argument for norm type. Swig does not like this and omits generating a
> > >> constructor for the derived types. According to
> > >> https://www.securecoding.cert.org/confluence/display/cplusplus/OBJ04-CPP
> > >>.+Prefer+not+to+give+virtual+functions+default+argument+initializers
> > >> virtual function should not provide default arguments at it tends to
> > >> "defeat polymorphism":
> > >>
> > >>
> > >> class Thing {
> > >>  public:
> > >>    virtual ~Thing();
> > >>    virtual void doItNTimes( int numTimes = 10 );
> > >>    virtual void doItThisHigh( double howHigh = 1.0 );
> > >>    // ...
> > >> };
> > >> class MyThing : public Thing {
> > >>  public:
> > >>    void doItNTimes( int numTimes );
> > >>    void doItThisHigh( double howHigh = 2.2 );
> > >>    // ...
> > >> };
> > >>
> > >> MyThing *mt = new MyThing;
> > >> Thing *t = mt;
> > >> t->doItNTimes(); // default of 10
> > >> mt->doItNTimes();  // compile time error!
> > >> t->doItThisHigh(); // default of 1.0!
> > >> mt->doItThisHigh(); // default of 2.2
> > >>
> > >>
> > >> Is this the intended behaviour anyhow?
> > >>
> > >> This is the proposed solution:
> > >
> > > Ouch, this was another, bad solution actually :)
> > >
> > > The proposed solution (from the link above) is roughly like this:
> > >
> > > class Base
> > > {
> > >    public:
> > >       void foo(double bar = 1.0) { foo_impl(bar); }
> > >    protected:
> > >       virtual void foo_impl(double bar);
> > > };
> > >
> > > class Sub: public Base
> > > {
> > >    protected:
> > >       virtual void foo_impl(double bar);
> > > };
> >
> > This is pretty ugly and I know that I'll forget about it within a day or
> > two. I suggest abandoning default arguments for norms if that fixes the
> > problem. We have a default argument for str(..), but I think that
> > PyDOLFIN has its own mechanism for this.
>
> Yes, this is correct. This was probably added to get around this issue. We can
> add workarounds for this case too, but it becomes quite cumbersome to do it
> for each method. Doing it for a fundamental base class as Variable maybe makes
> more sense?
>
> However note that following the report from Ola this will not work:
>
>   PETScMatrix *pm = new PETScMatrix();
>   ...
>   pm.src();  // Compile error
>
> where this would:
>
>   PETScMatrix *pm = new PETScMatrix();
>   ...
>   Variable * v = pm;
>   pm.str();
>
> This has probably not turned up as a problem as we are using str() indirectly
> through info().
>
> I can remove the default argument from
>
>   PETScMatrix::norm
>
> > Note that we have a related Blueprint,
> >
> >      https://blueprints.launchpad.net/dolfin/+spec/default-parameters-cpp
>
> I tried to link to a discussion for this blueprint but couldn't do that. We
> need to first register a bug on it to be able to connect it to a blueprint.
>
> Johan
>
> > Garth
> >
> > > Sorry about that...
> > >
> > > Ola
> > >
> > >> static double const minimumHeight = 1.0;
> > >> // ...
> > >> namespace XYZ {
> > >> class Thing {
> > >>  public:
> > >>    virtual ~Thing();
> > >>    virtual void doItThisHigh( double howHigh = minimumHeight );
> > >>    // ...
> > >> };
> > >> }
> > >> // ...
> > >> namespace XYZ {
> > >> static double const minimumHeight = 2.2;
> > >> class MyThing : public Thing {
> > >>  public:
> > >>    void doItThisHigh( double howHigh = minimumHeight );
> > >>    // ...
> > >> };
> > >> }
> > >>
> > >>
> > >> Resolving this issue also means getting the Python interface back on
> > >> track.
> > >>
> > >> Ola
> > >>
> > >>
> >
> > _______________________________________________
> > DOLFIN-dev mailing list
> > DOLFIN-dev@xxxxxxxxxx
> > http://www.fenics.org/mailman/listinfo/dolfin-dev
> >
> _______________________________________________
> DOLFIN-dev mailing list
> DOLFIN-dev@xxxxxxxxxx
> http://www.fenics.org/mailman/listinfo/dolfin-dev

Attachment: signature.asc
Description: Digital signature