Ravi Nori has posted comments on this change.
Change subject: engine: Add infrastructure code for removal of parameter classes
......................................................................
Patch Set 8:
(10 comments)
Will change to
<type>getParameter(parameter)
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/utils/VdcParameter.java
Line 37: }
Line 38: VdcParameter otherParam = (VdcParameter) other;
Line 39: return getName().equals(otherParam.getName())
Line 40: && getJavaType().equals(otherParam.getJavaType())
Line 41: && getGuid().equals(otherParam.getGuid());
will change
Line 42: }
Line 43:
Line 44: @Override
Line 45: public int hashCode() {
Line 45: public int hashCode() {
Line 46: int hash = 7;
Line 47: hash = 31 * hash + (this.name != null ? this.name.hashCode() :
0);
Line 48: hash = 31 * hash + (this.javaType != null ?
this.javaType.hashCode() : 0);
Line 49: hash = 31 * hash + (this.guid != null ? this.guid.hashCode() :
0);
will change
Line 50: return hash;
Line 51: }
Line 52:
Line 53: @Override
Line 57:
Line 58: public static VdcParameter create(String name, Class javaType,
String guidStr) {
Line 59: Guid guid = new Guid(guidStr);
Line 60: if (parametersMap.containsKey(guid)) {
Line 61: throw new InvalidParameterGuidException(name, guidStr,
parametersMap.get(guid));
ok
Line 62: }
Line 63: parametersMap.put(guid, new VdcParameter(name, javaType,
guid));
Line 64: return parametersMap.get(guid);
Line 65: }
Line 59: Guid guid = new Guid(guidStr);
Line 60: if (parametersMap.containsKey(guid)) {
Line 61: throw new InvalidParameterGuidException(name, guidStr,
parametersMap.get(guid));
Line 62: }
Line 63: parametersMap.put(guid, new VdcParameter(name, javaType,
guid));
ok
Line 64: return parametersMap.get(guid);
Line 65: }
Line 66:
Line 67: /**
Line 66:
Line 67: /**
Line 68: * Used by Json deserializer to retrieve the existing VdcParameter
Line 69: */
Line 70: public static VdcParameter getExisting(String guidStr) {
ok
Line 71: return parametersMap.get(new Guid(guidStr));
Line 72: }
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/utils/VdcParametersMap.java
Line 62: if (size() != otherParamsMap.size()) {
Line 63: return false;
Line 64: }
Line 65: for (Entry<VdcParameter, Object> entry : paramsMap.entrySet())
{
Line 66: Object otherValue = otherParamsMap.get(entry.getKey(),
entry.getKey().getJavaType());
I know what you are saying, but if I dont have this method many test that check
equality of parameters fails.
Line 67: if (otherValue == null) {
Line 68: return false;
Line 69: }
Line 70: Object thisValue = entry.getValue();
Line 78: @Override
Line 79: public int hashCode() {
Line 80: int hash = 7;
Line 81: final int prime = 31;
Line 82: for (Entry<VdcParameter, Object> entry : paramsMap.entrySet())
{
Will change and see how the junit test work
Line 83: hash = prime * hash + entry.getKey().hashCode();
Line 84: hash = prime * hash + entry.getValue().hashCode();
Line 85: }
Line 86: return hash;
Line 95: for (Entry<VdcParameter, Object> entry : paramsMap.entrySet())
{
Line 96: copy.put(entry.getKey(), entry.getValue());
Line 97: }
Line 98: return copy;
Line 99: }
I was always under the impression clone is java was broken
http://stackoverflow.com/questions/2356809/shallow-copy-of-a-map-in-java
Line 100:
Line 101: /**
Line 102: * Used by Json deserializer. Copy the contents of the map by
calling
Line 103: * put method which does the type checking
Line 102: * Used by Json deserializer. Copy the contents of the map by
calling
Line 103: * put method which does the type checking
Line 104: */
Line 105: public void setParamsMap(Map<VdcParameter, Object> paramsMap) {
Line 106: this.paramsMap = new HashMap<VdcParameter, Object>();
Here we are not just copying, we are using put method which does type checking.
I think it is required to do type checking what do you think?
Line 107: for (Entry<VdcParameter, Object> entry :
paramsMap.entrySet()) {
Line 108: put(entry.getKey(), entry.getValue());
Line 109: }
Line 110: }
....................................................
File
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/serialization/json/JsonObjectDeserializer.java
Line 101: private static class VdcParameterDeserializer extends
KeyDeserializer {
Line 102: @Override
Line 103: public VdcParameter deserializeKey(String key,
DeserializationContext ctxt) throws IOException {
Line 104: String[] tokens = key.split(";");
Line 105: return VdcParameter.getExisting(tokens[2]);
The key is in
name;javaType;guid
format, so token[2] is guid.
I considered making the key just guid, but readability of the JSON will be
effected. What do you think?
Line 106: }
Line 107: }
--
To view, visit http://gerrit.ovirt.org/20414
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6a0970e492c0eff561888a46b02e47645ff68fc3
Gerrit-PatchSet: 8
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Ravi Nori <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Barak Azulay <[email protected]>
Gerrit-Reviewer: Liran Zelkha <[email protected]>
Gerrit-Reviewer: Ravi Nori <[email protected]>
Gerrit-Reviewer: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: mooli tayer <[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
