Ravi Nori has posted comments on this change.

Change subject: engine: Add infrastructure code for removal of parameter classes
......................................................................


Patch Set 14:

(4 comments)

....................................................
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/VdcActionParametersBase.java
Yes definitely we should do that. We should even rename this class as it will 
not be parameters base anymore, it will be just plain parameters class. Not 
sure if we should do that now or as a final patch after we are done removing 
all parameter classes. What do you think?
Line 1: package org.ovirt.engine.core.common.action;
Line 2: 
Line 3: import java.util.ArrayList;
Line 4: import java.util.List;


....................................................
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/utils/InvalidParameterUuidException.java
Line 1: package org.ovirt.engine.core.common.utils;
Line 2: 
Line 3: public class InvalidParameterUuidException extends 
IllegalArgumentException {
Line 4: 
Line 5:     public InvalidParameterUuidException(String name, String uuid, 
String existingName) {
You mean also show the type of the conflicting parameter or UUID uuid in 
parameters list
Line 6:         super("Invalid value for uuid " + uuid + " for Parameter " + 
name +
Line 7:                 ". A parameter with the same uuid already exists " + 
existingName);
Line 8:     }
Line 9: 


....................................................
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/utils/VdcParameter.java
Line 10:     private final String name;
Line 11:     private final Class javaType;
Line 12:     private final UUID uuid;
Line 13:     private static final Map<UUID, VdcParameter> parametersMap = new 
HashMap<UUID, VdcParameter>();
Line 14:     private static final int VERSION = 0;
will do
Line 15:     private static final String VERSION_KEY = "version";
Line 16:     private static final String UUID_KEY = "uuid";
Line 17: 
Line 18:     private VdcParameter(String name, Class javaType, UUID guid) {


....................................................
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/utils/VdcParametersMap.java
It is a very good question and I considered doing that too. But we are giving 
this class part of map functionality and want to control the put method, it is 
ideal to do this by composition rather than inheritance (extends). 

This is a recommend way of doing things according to Joshua Bloch's Effective 
Java "Item 16 : Favor composition over inheritance". He explains why it would 
be problematic to just extend from Collections when we want to control the 
add/put functionality.
Line 1: package org.ovirt.engine.core.common.utils;
Line 2: 
Line 3: import java.io.Serializable;
Line 4: import java.util.Collections;


-- 
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: 14
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Ravi Nori <rn...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com>
Gerrit-Reviewer: Liran Zelkha <lzel...@redhat.com>
Gerrit-Reviewer: Ravi Nori <rn...@redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com>
Gerrit-Reviewer: mooli tayer <mta...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to