Re: [Getfem-users] Commit 4120

[logari81]

ok something like:

  mesh_fem& mesh_fem::operator=(const mesh_fem &mf) {
    linked_mesh_ = 0;
    if (mf.linked_mesh_) {
      init_with_mesh(mf.linked_mesh(), mf.get_qdim());
      for (size_type i=0; i < mf.linked_mesh().nb_convex(); i++)
        this->set_finite_element(i, mf.fem_of_element(i));
    } else
      dof_enumeration_made = false;
    return *this;
  }

  mesh_fem::mesh_fem(const mesh_fem& mf)
  {
    this->operator=(mf);
  }

seems to work correctly with respect to the issues discussed so far. 
However, I can still imagine that the new copy constructor would 
possibly cause other regressions. E.g. it will not copy the reduction 
and extension matrices and maybe some users have already code that 
relies on the assumption that these matrices are copied.

Shouldn't we go through all class members and try to copy as much as 
possible? The copy constructor of the mesh_fem class is a very 
fundamental point in getfem, we should make sure that the new 
constructor is as compatible as possible to the previous behavior.

Regards

Kostas

On 07/25/2012 05:27 PM, logari81 wrote:
> Dear Andriy,
>
> ok your intention with this commit is clear and solution 2 is easy to 
> implement. However there is still an issue with your copy constructor. 
> It will not copy an uninitialized mesh_fem:
>
> getfem::mesh_fem mf1;
> getfem::mesh_fem mf2(mf1);
>
> of course a user will not do this but std::vector<mesh_fem>::resize() 
> will do.
>
> I will try to fix this and maybe I will have to ask you again to do 
> some review/testing. It would be nice to have a fix before 4.2 is 
> released in the next days.
>
> Kostas
>
> P.S.: thanks Roman for your comments
>
> On 07/25/2012 02:41 PM, Andriy Andreykiv wrote:
> > Dear Kostas,
> >
> > I think this was me who implemented copy constructor for mesh_fem
> > recently and "privatized" operator=.
> > The reason is that there was no copy constructor before, so the copy
> > operation was generated by the compiler
> > and it wasn't done correctly. I made the operator "=" private to point
> > out that if it's not, the default operator might
> > behave not the way you expect.
> >
> > You can have two solutions here: 1)quick and dirty one - remove my
> > private declarations. Expect problems when
> > you copy one mesh_fem to the other and one of them is local variable
> > that goes out of scope, while the other is global (the global one will
> > get unvalid context)
> > 2)The thorough solution (but still as quick as this email :) ) is to
> > still make operator "=" public but define it similarly to the copy
> > constructor.
> >
> > Best regards,
> >                          Andriy
> >
> >
> > On 25 July 2012 13:18, logari81 <address@hidden> wrote:
> > > Hi,
> > >
> > > I have some issues with the change introduced in revision 4120. 
> > > Making the =
> > > operator for the mesh_fem class private, will not let you resize or 
> > > fill a
> > > std::vector<getfem::mesh_fem> because the copy operator is required.
> > >
> > >
> > > Is it possible to rework this commit so that it doesn't cause this
> > > restriction?
> > >
> > > Best regards
> > >
> > > Kostas
> > >
> > >
> > > _______________________________________________
> > > Getfem-users mailing list
> > > address@hidden
> > > https://mail.gna.org/listinfo/getfem-users