Martin Peřina has posted comments on this change.

Change subject: core: Provide copy constructor for FenceAgent
......................................................................


Patch Set 4:

(2 comments)

https://gerrit.ovirt.org/#/c/38163/4/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/FenceAgent.java
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/FenceAgent.java:

Line 55:     public FenceAgent() {
Line 56:         this(null);
Line 57:     }
Line 58: 
Line 59:     public FenceAgent(FenceAgent other) {
> Consider using "clean code" technique of providing a 
Well, currently I don't see a benefit to replace classic copy constructor with 
static copyOf method, but I will try to read something about it.
Line 60:         if (other != null) {
Line 61:             this.id = other.id;
Line 62:             this.hostId = other.hostId;
Line 63:             this.order = other.order;


Line 68:             this.password = other.password;
Line 69:             this.encryptOptions = other.encryptOptions;
Line 70:             this.options = other.options;
Line 71:             if (other.optionsMap != null) {
Line 72:                 this.optionsMap = new HashMap<>();
> Is that really a copy or are you want to create a FenceAgent that reference
AFAIK copy constructor is only used in backend code. And I really want to have 
deep copy, because I need to alter options of copied object.

About serialization/deserialization case:

1. Doesn't it hide what should be obvious? I mean that we really want a copy
2. Possible performance issues?
Line 73:                 for (Map.Entry<String, String> entry : 
other.optionsMap.entrySet()) {
Line 74:                     this.optionsMap.put(entry.getKey(), 
entry.getValue());
Line 75:                 }
Line 76:             }


-- 
To view, visit https://gerrit.ovirt.org/38163
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I382e01d35af79a3600e34e9a74757c2eba831482
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Martin Peřina <[email protected]>
Gerrit-Reviewer: Eli Mesika <[email protected]>
Gerrit-Reviewer: Martin Peřina <[email protected]>
Gerrit-Reviewer: Ori Liel <[email protected]>
Gerrit-Reviewer: Oved Ourfali <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to