Martin Peřina has posted comments on this change. Change subject: tools: Refactor model and DB mapping of eventNotificationMethods. ......................................................................
Patch Set 16: (5 comments) .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddEventSubscriptionCommand.java Line 91: boolean retval = false; Line 92: for (event_subscriber eventSubscriber : subscriptions) { Line 93: if (subscriberId.equals(eventSubscriber.getsubscriber_id()) Line 94: && StringUtils.equals(eventSubscriber.getevent_up_name(), eventName) Line 95: && eventSubscriber.getevent_notification_method().equals(eventNotificationMethod)) { Please use == instead of equals() for enums. Line 96: retval = true; Line 97: break; Line 98: } Line 99: } .................................................... File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/EventNotificationMethods.java Line 1: package org.ovirt.engine.core.common; Line 2: Line 3: public enum EventNotificationMethods { I would prefer singular here (EventNotificationMethod). And since you delete class with same name in the patch, I don't see a problem with it. Line 4: EMAIL .................................................... File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/event_subscriber.java Line 24: result = prime * result + ((id.eventUpName == null) ? 0 : id.eventUpName.hashCode()); Line 25: result = prime * result + ((methodAddress == null) ? 0 : methodAddress.hashCode()); Line 26: result = prime * result + ((id.eventNotificationMethod == null) ? 0 : id.eventNotificationMethod.hashCode()); Line 27: result = prime * result + ((id.subscriberId == null) ? 0 : id.subscriberId.hashCode()); Line 28: result = prime * result + ((id.tagName == null) ? 0 : id.tagName.hashCode()); Why aren't you using hashCode() method you defined in event_subscriber_id? Line 29: return result; Line 30: } Line 31: Line 32: @Override Line 44: return (ObjectUtils.objectsEqual(id.eventUpName, other.id.eventUpName) Line 45: && ObjectUtils.objectsEqual(methodAddress, other.methodAddress) Line 46: && ObjectUtils.objectsEqual(id.eventNotificationMethod, other.id.eventNotificationMethod) Line 47: && ObjectUtils.objectsEqual(id.subscriberId, other.id.subscriberId) Line 48: && ObjectUtils.objectsEqual(id.tagName, other.id.tagName)); Why aren't you using equals() method you defined in event_subscriber_id? Line 49: } Line 50: Line 51: public event_subscriber(String event_up_name, EventNotificationMethods eventNotificationMethod, Line 52: Guid subscriber_id, String tagName) { .................................................... File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/event_subscriber_id.java Line 38: } Line 39: event_subscriber_id other = (event_subscriber_id) obj; Line 40: return (ObjectUtils.objectsEqual(subscriberId, other.subscriberId) Line 41: && ObjectUtils.objectsEqual(eventUpName, other.eventUpName) Line 42: && ObjectUtils.objectsEqual(eventNotificationMethod, other.eventNotificationMethod) Please use == instead of equals() for enums Line 43: && ObjectUtils.objectsEqual(tagName, other.tagName)); Line 44: } -- 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: 16 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: mooli tayer <mta...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com> Gerrit-Reviewer: Eli Mesika <elimes...@gmail.com> Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com> Gerrit-Reviewer: Martin Peřina <mper...@redhat.com> Gerrit-Reviewer: Moti Asayag <masa...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@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