In C++ I can e.g. do this:Arguments:
It is still not absolutely clear what you mean by consistent. However, it may not be important as you have detailed two implementations and I understand that.
Code: Select all
GeomPoint* pnt = new GeomPoint();
pnt->setPoint(x, y, z,);
// create a copy
GeomPoint* cpy = new GeomPoint();
cpy->setHandle(Handle_Geom_CartesianPoint::DownCast(pnt->handle()));
...
Now look at this:I do not favour the second implementation (createNewTagId), if only, because it implies (constructor assigning a new tagid + clone assigning a tag id + createNewTagId assigning a new tagid), unless there is no other way.
Code: Select all
void function(Geometry* geom)
{
Geometry* copy = geom->clone();
...
}
I disagree. The clone is an independent copy which you can modify afterwards. So, with clone() using the same tag id we end up with two different geometries that have the same tag id.The thing is that before my changes the clone function was producing a clone, if we implement our solution 1, clone will not produce a clone, if only, because the tag is different. I think this is confusing and should be avoided.
Unless I missed something then the use cases where the clone must have the same tag id as the original is only when either the clone or the original is only hold temporarily. To make this clearer I have exposed the getTag to Python:
Code: Select all
doc=App.newDocument()
sketch=doc.addObject("Sketcher::SketchObject","Sketch")
c=Part.Circle()
n=c.copy() # uses clone() of the C++ class!
n.Center.x=5.0
sketch.addGeometry(c)
sketch.addGeometry(n)
sketch.Geometry[0].Tag
sketch.Geometry[1].Tag # this is the same tag id but a different geometry!
But this is on a much higher level and only dedicated to the sketcher. I am talking about the lower level API which in itself should be consistent.I was referring to clone and copy as concepts more than member functions.
As said above the clone with the same tag id usually only happens where either the clone or the original will be deleted very soon.In the sketcher I need copies (different tagid) and clones (same tagid).
Since you can create a copy or a clone with the same API function the question to me is what is easier to do:
* create a clone with the same tag id and create a new one afterwards
* create a copy with a different tag id and support a way that it has the same tag id
I haven't checked your code. Just in case here you don't have to make copy virtual and reimplement it for all sub-classes. It suffices to have a non-virtual method like:how do I handle now that tag? It should be uniform. Already as part of Carbon Copy, I have a separate commit with a copy() member in Geometry, like clone() but without assigning the tag. This is used in carbon copy, but will be used for example in array, symmetric element.
Code: Select all
Geometry* Geometry::copy()
{
Geometry* cpy = clone();
cpy->tag = ...
return cpy;
}
In this case the assigntagid is not needed any more. The programmer then has to just use the correct method.Having two separate well-documented copy methods, clone() and copy(), and in addition your "assigntagid (geometry *) for the sethandle and handle constructors".
Btw, after merging your branch I have added the method createNewTag. If we come to the conclusion we don't need it for client programmers it suffices to make it protected.
Before continuing can you rebase it on latest master please?Question: Do you have anything against me adding the commits of carbon copy on this pull request? That it will avoid that I have to make adaptations to both branches that will finally cancel out, as I have geometry changes in both.