Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate IBDO.clone()/BDO.clone() in preparation for deletion per Sonar warning #707

Merged

Conversation

jdcove2
Copy link
Collaborator

@jdcove2 jdcove2 commented Feb 22, 2024

Sonar has the below warning for IBaseDataObject.clone() and BaseDataObject.clone(). There is an IBaseDataObjectHelper.clone(...) method that will do both the partial clone that BaseDataObject.clone() does along with a full clone. Therefore, the clone methods on IBaseDataObject and BaseDataObject are not needed. They are being deprecated in this PR and then will be deleted in a future PR.

--

WARNING:
Remove this "clone" implementation; use a copy constructor or copy factory instead.

--

REASON:
Many consider clone and Cloneable broken in Java, largely because the rules for overriding clone are tricky and difficult to get right, according to Joshua Bloch:

Object’s clone method is very tricky. It’s based on field copies, and it’s "extra-linguistic." It creates an object without calling a constructor. There are no guarantees that it preserves the invariants established by the constructors. There have been lots of bugs over the years, both in and outside Sun, stemming from the fact that if you just call super.clone repeatedly up the chain until you have cloned an object, you have a shallow copy of the object. The clone generally shares state with the object being cloned. If that state is mutable, you don’t have two independent objects. If you modify one, the other changes as well. And all of a sudden, you get random behavior.

A copy constructor or copy factory should be used instead.

@jpdahlke jpdahlke added this to the v8.0.0-M15 milestone Feb 27, 2024
@jpdahlke jpdahlke added the tech-debt Low-impact cleanup and upkeep label Feb 27, 2024
@jpdahlke jpdahlke merged commit c24aac8 into NationalSecurityAgency:master Feb 28, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tech-debt Low-impact cleanup and upkeep
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants