Model copying concerns

[Robert Carlsen]

Why is it done this way:

...

old_model...

...

FacilityModel* new_model = new FacilityModel();

new_model->copy(old_model);

instead of this way:

...

old_model...

...

FacilityModel* new_model = old_model.copy();

The latter seems much more intuitive/clean to me.  Also - why do we need to have a copyFresh and a copy method?

[Katy Huff]

There's a big discussion concerning that. 

It's like... a deep copy construction for the initialization step.

If you and Matt can talk in person about it, that will be cleaner than any explanation I could give over the list. 

He's well versed in this topic.

--
http://homepages.cae.wisc.edu/~khuff/

[Steve Jackson]

A more C++-y way to do an operation like this would be to use a copy constructor. Then you'd get the following idiom:

> FacilityModel* new_model = new FacilityModel( old_model );

When I see the 'new' operator, it's obvious that a new object has been born on the heap, and that the pointer should be delete-d later. An assignment operator (operator=) can work similarly.

I notice that Model::copy() is a virtual function. If the arrangement of your class hierarchy make copying by copy constructors impractical, you may need to stick with a copy function. (Read about "virtual constructors" [1] for some of the subtleties involved.) This is basically what Robert is recommending.

My own tendency is to prefer the existing idiom. The argument against

> FacilityModel* new_model = old_model.copy();

is that it isn't as clear, without extra documentation, whether .copy() returned a new-ly allocated object, or who is responsible for deleting that object later. If you had a function like that, I'd recommend naming it something like make_new_deep_copy() or clone() [2], to help the reader realize that a new object is being allocated.
~S

[1]: http://www.parashift.com/c++-faq-lite/virtual-functions.html#faq-20.8 (You'll notice me referencing this site often-- the entire C++ FAQ is very useful!)

[2]: I believe clone() is more common as the name of a virtual copy constructor than copy() is. This is probably due in part to the use of clone() for this purpose in Java.

[Robert Carlsen]

My own tendency is to prefer the existing idiom.  The argument against

> FacilityModel* new_model = old_model.copy();

is that it isn't as clear, without extra documentation, whether .copy() returned a new-ly allocated object, or who is responsible for deleting that object later.  If you had a function like that, I'd recommend naming it something like make_new_deep_copy() or clone() [2], to help the reader realize that a new object is being allocated.

 I like 'clone' - it sounds researchish.  I agree that 'copy' is ambiguous.  The problem, Steve, is that these models are dynamically loaded from shared object files.  So we don't 'know' the name of the class in advance (which is the name of the copy constructor).   I had a discussion with Matt and I believe that switching to an old_model.deepcopy() idiom will actually eliminate the need for two separate copy methods (and thus reduce complexity).