Martin Mucha has posted comments on this change. Change subject: core: AuditLogType.UNASSIGNED should not be logged ......................................................................
Patch Set 3: (1 comment) http://gerrit.ovirt.org/#/c/33359/3/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java: Line 1128: return severity; Line 1129: } Line 1130: Line 1131: public boolean shouldBeLogged() { Line 1132: return this != AuditLogType.UNASSIGNED; > IMHO, it should be implemented differenty - yes, you're right; that's common and correct style to add parameter to enum value. I think of it, but opted for not use it because: * to be OO correct, parameter in constructor should be related to all, or at least to most values (example: I cannot extend class just because I want 1 method and remaining 99 would not behave correctly or would not make sense). From this point of view, 'shouldBeLogged' isn't parameter of enum, since currently only for one value it has different value. I.e. it's probably not a property of this type (we know it for sure since this is a hack and this should be implemented elsewhere) * there are approx 990 instances which all returns false, so implementing it via field we're literally throwing out one kb of memory for nothing. For these reasons I decided to implement it instead of 'declarative way' via function which computes this property. Do you still like the 'declarative' way better? If so, I'll change it. Line 1133: } Line 1134: Line 1135: public static AuditLogType forValue(int value) { Line 1136: return mappings.get(value); -- To view, visit http://gerrit.ovirt.org/33359 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I97bf90264d1a1b3d8ea571daa9346d1f6b6d18d1 Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Mucha <mmu...@redhat.com> Gerrit-Reviewer: Lior Vernia <lver...@redhat.com> Gerrit-Reviewer: Martin Mucha <mmu...@redhat.com> Gerrit-Reviewer: Moti Asayag <masa...@redhat.com> Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org 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
