[Dune] [Dune-Commit] dune-localfunctions r943 - trunk/dune/localfunctions/test

[Martin Nolte]

Hi Jö,

let me get this straight. Your code example shows that the facade does not 
guarantee a method to be present on the implementation just because the facade 
implements it -- granted. But there is a subtle difference: Let's assume we 
would call bar instead of foo in the main program of your example. With the 
facade, the error occurs in Facade::bar, while without the facade the error 
occurs in the main program (read: user code). So, with the facade, the user 
tell the difference between an error in his code and an error in the grid 
implementation.

As to the fact that the facades prohibit access to non-interface 
functionality, I'm all on your side. That's exactly why I suggested a public 
method impl(). This way, the user can see whenever non-interface functionality 
is called:

Interface method:     geometry.global( x, y );
Non-Interface method: geometry.impl().coordwiseGlobal( x[ 0 ], x[ 1 ], y );

This way, the user really sees that non-interface functionality is called, 
which might not be present in all grid implementations. If we remove the 
facade, the user cannot see this anymore. But, of course, it boils down to 
some documentational stuff.

Finally, the incomplete type problem is as follows. When I wrote my first DUNE 
grid, I tried to extract the types from the const Grid provided to, e.g., the 
geometry. This failed due to the grid being an incomplete type. Today I know 
that I have to use remove_const to get around this. But the lesson I learned 
is: Think whether it is necessary to extract stuff from a template argument 
which is not a traits class.

Just by the way, I wanted to specialize as follows:

template< int mydim, int cdim, class GridImp, template< int, int, class > Impl >
struct MyObjectWrapper< Geometry< mydim, cdim, GridImp, Impl > > { ... };

template< int codim, int dim, class GridImp, template< int, int, class > Impl >
struct MyObjectWrapper< Entity< codim, dim, GridImp, Impl > > { ... };

You can't do this trick without the Facade. But your example of specializing 
over the codimension is a really good one, too.

I hope, this clarifies my point of view a bit.

Best,

Martin


On 01/03/2011 12:28 AM, Jö Fahlke wrote:
> Am Sun,  2. Jan 2011, 17:01:46 +0100 schrieb Martin Nolte:
>> as far as I know, the issue whether the implementation should be accessible was
>> discussed on the developer meeting and it was decided that additional
>> functionality should _not_ be accessible to the user. For testing purposes, a
>> special macro shall be defined to grant access to getRealImp().
>>
>> While I did wish the getRealImp() method (my preferred name would have been
>> impl(), though) to be public, this is not going to happen. So we have to live
>> with this decision :-(.
>
> Don't get me wrong, I do think having non-interface functionality on something
> like the Geometry is a bad idea.  But I also think that inhibiting it using
> this facade wrapper as we do it currently is going a bit too far.
>
>>                          On the other hand, having the facade inbetween the user
>> and the implementation is a very useful safeguard.
>
> Against what are we safeguarding here exactly?  I always thought we wanted to
> ensure that a certain interface exists, but consider for instance the
> following code
>
> ======================================================================
> struct Engine {
>    void foo() { std::cout<<  "Engine::foo()"<<  std::endl; }
> };
>
> template<class Engine>
> class Facade {
>    Engine engine;
>
> public:
>    void foo() { engine.foo(); }
>    void bar() { engine.bar(); }
> };
>
> int main() {
>    Facade<Engine>  facade;
>    facade.foo();
> };
> ======================================================================
>
> This will compile without problem, even though it looks like Facade::bar() is
> expecting a method Engine::bar(), which does not exist.
>
>>                                                     Moreover, this way you can
>> specialize for a Geometry or Entity in a generic way.
>
> Depends what you want to specialize for.  You can specialize for mydim, cdim
> or ctype using enable_if even without the Dune::Geometry template.  But you
> are right in so far as it is probably easier if the parameter you want to
> specialize for is a template parameter of Dune::Geometry (which is not the
> case for ctype currently).
>
> Specializing for GeometryImp is either easy or impossible both with or without
> Dune::Geometry -- it is impossible if the GeometryImp template is a member of
> some other template class.
>
>>                                                        Finally, the MockGeometry
>> is as useless as the Geometry and I think you yourself liked to have a simple
>> interface around the complicated BasicGeometry.
>
> Actually, I wanted to have some class that provides the same interface as a
> Dune::Geometry (after construction), so I could use it as a drop-in
> replacement.  Since BasicGeometry has different member types and the Jacobian
> types are not field matrices, I cannot use it directly.  I will happily use a
> Dune::Geometry once it no longer requires a Grid.  But I don't see
>
>> As for mydim, cdim and ctype, it is true that the implementation must know them.
>> But checking the interface is far easier, if they are specified as template
>> arguments and this tiny bit of redundancy does not harm in my opionion. It also
>> avoids problems due to incomplete templates.
>
> I looked at the code and I do not understand how it would get more complicated
> without the template arguments for mydim and cdim, once GeometryImp is changed
> from a template to a type.  In dune/grid/common/geometry.hh you simply extract
> mydim, cdim and ctype from GeometryImp instead of using the template
> arguments.  In dune/grid/test/checkgeometry.cc it is basically the same.  Did
> I miss any check?
>
> Could you please elaborate on "problems due to incomplete templates"?  You
> have lost me here...
>
>> Finally, I agree with Christian that it would be a good idea to introduce
>> additional namespaces. But this cannot be done before the release and, in my
>> opionion, should be done when adding the new module.
>
> Agreed.
>
> Bye,
> Jö.
>
>
>
>
> _______________________________________________
> Dune mailing list
> Dune at dune-project.org
> http://lists.dune-project.org/mailman/listinfo/dune

-- 
Martin Nolte <nolte at mathematik.uni-freiburg.de>

Universität Freiburg                                   phone: +49-761-203-5630
Abteilung für angewandte Mathematik                    fax:   +49-761-203-5632
Hermann-Herder-Straße 10
79104 Freiburg, Germany