Give DrawnElement a copy constructor #4763
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
To enable this, `Drawable` now has a `copy()` method, which is consistently implemented in child classes as delegating to each implementation's copy constructor. The copy constructor always keeps the same ID, but callers can now modify the ID through a new method `Drawable.setId(GUID)` for those cases where the ID needs to be given a differnt ID from the original.
If the flag to keep IDs is not set, then new GUIDs are created and written to the copies. Otherwise, drawing IDs are kept the same.
Now each implementation of `Drawable` has its own `fromDto()` method that focuses only on that implementation's needs. `Drawable.fromDto()` simply delegates to each implementation based on the type case of the parent DTO.
kwvanderlinde
force-pushed
the
bugfix/2433-copy-drawings-when-copying-maps
branch
from
5b4e34a
to
f452a8f
Compare
thelsing
approved these changes
May 6, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Identify the Bug or Feature request
Fixes #2433
Properly fixes #4526
Description of the Change
DrawnElementnow has a copy constructor that will copy the underlyingDrawableandPen.Penalready has a copy constructor, butDrawabledid not have anything for this. So a newDrawable.copy()method is added, with concrete child classes deciding how they should be copied. For practical reasons, this is always implemented via copy constructors.The above is mostly straightforward, but
ShapeDrawableis a unique case. It wraps a generalShape, but not allShapeimplementations are easily copied. So for this we do case work on the type of the shape: the common cases ofArea,Polygon, andRectangularShape(coveringRectangle,Ellipse2Dand more), are handled explicitly. There shouldn't be other type encountered inShapeDrawable, but to cover our bases we simply do a shallow a copy if some unknownShapeimplementation is found.In the copy constructor of
Zone, each drawing is copied using the newDrawnElementcopy constructor. IfkeepIDsisfalse, a new GUID will also be generated and set for each copy.Finally, I took the opportunity to refactor
Drawable.fromDto()so that each concrete class has its ownfromDto()method, withDrawable.fromDto()doing nothing but delegating base on type case. I find this easier to read with thetoDto()andfromDto()methods for each type sitting right next to each other.Possible Drawbacks
Should be none.
Documentation Notes
N/A
Release Notes
This change is