Skip to content

Added a deepcopy mechanism#392

Merged
pariterre merged 1 commit into
pyomeca:devfrom
pariterre:dev
Apr 13, 2026
Merged

Added a deepcopy mechanism#392
pariterre merged 1 commit into
pyomeca:devfrom
pariterre:dev

Conversation

@pariterre
Copy link
Copy Markdown
Member

@pariterre pariterre commented Apr 13, 2026

This change is Reviewable

@petersteneteg
Copy link
Copy Markdown

Thank you very much! This solves my use case.

Reading the PR I wondered why you added a clone instead of just a copy constructor?
A clone function is usually only needed if you are trying to create a copy of a polymorphic class using a reference or pointer to a base of the class you want to copy. And then your clone function has to return a value on the heap.

In your case you return by value. which means that the clone function is just a the same as a copy constructor but with a different name?

Is there any reason not to just use the copy constructor, and define it in the cases where the default will create shallow copies?

Or are the shallow copies intentional, and needed for some use case?

As a C++ dev Im used to value based design and the rule of 5/3/0 etc. But I can understand if you have different needs, Im mostly just curious about the reasoning.

@pariterre
Copy link
Copy Markdown
Member Author

Reading the PR I wondered why you added a clone instead of just a copy constructor?

I must admit there is not much thought.. Since I was not sure how deep I could test this, I just decided the worst case scenario was the best way to do it, and since I have two binders, including a Python one, I was worried it would conflict if I was just adding a new constructor... clone (or deepcopy) method was therefore my goto... But again, it probably would have work (except for the internal shared_ptr in one of the class.

That said, if you have a better way to do it, feel free to PR, I am not specifically trained in C++, it is mostly self teached!

:)

@pariterre pariterre merged commit 9ea41ab into pyomeca:dev Apr 13, 2026
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants