[Dune] [Dune-Commit] dune-istl r929 - trunk/istl

[Markus Blatt]

Hi,

I have a few comments on this patch.

On Mon, Jul 28, 2008 at 09:49:55AM +0200, sander at dune-project.org wrote:
> Modified:
>    trunk/istl/scaledidmatrix.hh
> Log:
> - Disable operator K
> - add cast to FieldMatrix
> - add operator[]
> 
>  
> +    /** \brief Cast to FieldMatrix
> +     * Might be inefficient, but operator= has to be a member of FieldMatrix
> +     * */
> +    operator FieldMatrix<K,n,n>() const
> +    {
> +        FieldMatrix<K, n, n> fm = 0.0;
> +        for(int i=0; i<n; ++i)
> +            fm[i][i] = p_;
> +        return fm;
> +    }
> +

This cast operator returns a value in contrast to a pointer or
reference or just a copy of internal data.

One usually assumes that no temporaries are created when using
type conversion operators. In contrast now memory is allocated on the
stack each time the type conversion occurs. 

The type conversion operator  makes it possible to call all methods
for a FiedMatrix with a ScaledIdMatrix.  Resulting in programs being slower
with ScaledIdMatrix than when using FieldMatrix. This might come as a
surprise to users.

Why and where is this operator needed? 
What about making it explicit?

Cheers,

Markus