Moti Asayag has posted comments on this change.
Change subject: tools: Remove cumbersome NotificationMethod abstraction.
......................................................................
Patch Set 11:
(6 comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/EventSubscriptionCommandBase.java
Line 44: protected boolean
ValidateNotificationMethod(EventNotificationMethod eventNotificationMethod,
Line 45: event_subscriber
event_subscriber, DbUser user) {
Line 46: boolean retValue = true;
Line 47:
Line 48: switch (eventNotificationMethod) {
let's face it: a switch with one case and one default is basically if-else ;-)
Line 49: case EMAIL:
Line 50: String mailAdress =
(StringUtils.isEmpty(event_subscriber.getmethod_address())) ? user.getEmail()
Line 51: : event_subscriber.getmethod_address();
Line 52:
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/EventNotificationMethod.java
Line 5:
Line 6: public enum EventNotificationMethod {
Line 7: EMAIL(0);
Line 8:
Line 9: private int intValue;
i guess methodId or just id will describe this field better
Line 10: private static Map<Integer, EventNotificationMethod> mappings;
Line 11:
Line 12: static {
Line 13: mappings = new HashMap<Integer, EventNotificationMethod>();
Line 6: public enum EventNotificationMethod {
Line 7: EMAIL(0);
Line 8:
Line 9: private int intValue;
Line 10: private static Map<Integer, EventNotificationMethod> mappings;
is this mapping really required ? there is only single element...
we should drop and if we introduce new type - i think a switch clause will make
more sense.
Line 11:
Line 12: static {
Line 13: mappings = new HashMap<Integer, EventNotificationMethod>();
Line 14: for (EventNotificationMethod value : values()) {
Line 23: public int getValue() {
Line 24: return intValue;
Line 25: }
Line 26:
Line 27: public static EventNotificationMethod forValue(int value) {
s/value/methodId or just id
Line 28: EventNotificationMethod eventNotificationMethod =
mappings.get(value);
Line 29: if (eventNotificationMethod == null){
Line 30: throw new IllegalArgumentException("Unsupported event
notification method value: " + value);
Line 31: }
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/event_subscriber_id.java
Line 10: private static final long serialVersionUID = 9035847334394545216L;
Line 11:
Line 12: Guid subscriberId;
Line 13: String eventUpName;
Line 14: EventNotificationMethod notificationMethod;
why package modifier ? any reason not to define it as private and expose proper
getter and setter ?
(i'd expect the rest of the properties also to be defined as private and
encapsulated rather than a direct access to them).
Line 15: String tagName;
Line 16:
Line 17: @Override
Line 18: public int hashCode() {
....................................................
File
backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/NotificationMethodsMapper.java
Line 19: public EventSender getEventSender(EventNotificationMethod method) {
Line 20: return eventSenders.get(method);
Line 21: }
Line 22:
Line 23: public EventSender getEventSender(int intValue) {
s/intValue/methodId ?
Line 24: return
eventSenders.get(EventNotificationMethod.forValue(intValue));
Line 25: }
--
To view, visit http://gerrit.ovirt.org/22135
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b71c4e78bbdca3d02d2ac4ef419b9d3d7d58761
Gerrit-PatchSet: 11
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: mooli tayer <[email protected]>
Gerrit-Reviewer: Barak Azulay <[email protected]>
Gerrit-Reviewer: Eli Mesika <[email protected]>
Gerrit-Reviewer: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Martin Peřina <[email protected]>
Gerrit-Reviewer: Moti Asayag <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[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
