kicad-developers team mailing list archive

[Wayne Stambaugh <stambaughw@xxxxxxxxx>]

On 5/26/2016 10:38 AM, Maciej Sumiński wrote:
> On 05/26/2016 09:46 AM, jp charras wrote:
>> Le 26/05/2016 à 08:59, Maciej Sumiński a écrit :
>>> Thank you Cirilo. I am trying to be really careful here, that is why I
>>> am asking for more details. I would like to make the code simpler, so
>>> there are no more doubts about whether to use operator = or Copy() (for
>>> some classes it is exactly the same code).
>>>
>>> For the scenario you described, there is EDA_ITEM::Clone() method.
>>> Copy() is defined only for a few classes, and others are copied using
>>> operator=, so it is a bit confusing to me.
>>>
>>> Regards,
>>> Orson
>>
>> Usually, Copy() is used when you do not want to copy all members.
>> Typically when you do not want to change Pback, Pnext, m_Parent, time stamp and a few other pointers
>> and flags.
>>
>> If these members are reinitialized (or have the right value) after copy, using Copy() or = (for
>> classes which do not define Copy() and can be copied by = ) can give the same result.
> 
> Hi Jean-Pierre,
> 
> Thank you for the explanations, it is quite clear now. The code is
> currently a bit incoherent (e.g. ZONE_CONTAINER::Copy() or
> TEXT_PCB::Copy()), I presume you will not mind if I fix it?
> 
> Regards,
> Orson

Hey Orson,

Be extremely careful here.  Getting the various copy functions wrong can
break things in subtle ways that are difficult to debug.  Also be really
careful with Clone() as it is used by the boost pointer containers.
Take a look at the comment near the end of include/base_struct.h and the
Boost pointer container documentation for how this all works.  Of course
you could always convert the boost pointer containers to c++ containers
using one of the pointer management objects but that would not be a
trivial task.

That being said, I don't have any issues with using the assignment
operator for copying objects.  It certainly would make the code easier
to understand.  It will most likely be a big effort to implement this
because you will have to find all of the places in the code where Copy()
is used and change it to use the assignment operator copying properly.
Changing this incrementally will most likely not be possible.

Cheers,

Wayne