-
Notifications
You must be signed in to change notification settings - Fork 122
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 IBaseDataObjectHelper.clone() that allows partial clones. #1018
base: main
Are you sure you want to change the base?
Deprecate IBaseDataObjectHelper.clone() that allows partial clones. #1018
Conversation
* @return the clone of the IBaseDataObject passed in. | ||
*/ | ||
public static IBaseDataObject clone(final IBaseDataObject iBaseDataObject) { | ||
return clone(iBaseDataObject, true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a fan having the new overload reference the very method we're deprecating. Maybe we should refactor the existing one by extracting two supporting methods that both overloads can use. Something like the following signatures:
private static copyBasicFields(final IBaseDataObject src, final IBaseDataObject dest) {
// body would be current lines 66-87
}
private static copyNonBasicFields(final IBaseDataObject src, final IBaseDataObject dest) {
// body would be current lines 90-106
}
With that change, this overload would strip down to the following:
public static IBaseDataObject clone(final IBaseDataObject src) {
Validate.notNull(src, "Required: src not null");
IBaseDataObject dst = new InternalIdBaseDataObject(src.getInternalId());
copyBasicFields(src, dst);
copyNonBasicFields(src, dst);
retrun dst;
}
The original now-deprecated overload would become this:
public static IBaseDataObject clone(final IBaseDataObject src, final boolean fullClone) {
Validate.notNull(src, "Required: src not null");
IBaseDataObject dst = fullClone ? new InternalIdBaseDataObject(src.getInternalId()) : DataObjectFactory.getInstance();
copyBasicFields(src, dst);
if (fullClone) {
copyNonBasicFields(src, dst);
}
retrun dst;
}
After the deprecated overload is removed, we can collapse those private methods into the remaining clone
method
/** | ||
* Clones an IBaseDataObject equivalently to emissary.core.BaseDataObject.clone(), which duplicates some attributes. | ||
* | ||
* A "fullClone" duplicates all attributes. | ||
* | ||
* @deprecated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* @deprecated | |
* @deprecated prefer {@link #clone(IBaseDataObject)} |
* @param iBaseDataObject the IBaseDataObject to be cloned. | ||
* @param fullClone specifies if all fields should be cloned. | ||
* @return the clone of the IBaseDataObject passed in. | ||
*/ | ||
@Deprecated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Deprecated | |
@Deprecated(forRemoval = true) |
This PR deprecates the IBaseDataObjectHelper.clone() method that allows for partial clones. Partial clones were implemented to allow cloning that matched the now deprecated BaseDataObject.clone() method. A search of the code bases did not show any instance where IBaseDataObjectHelper.clone() was called for a partial clone except for in the IBaseDataObjectHelperTest class. Therefore, only full clones seem to be needed and partial clones should be removed.