Martin Mucha has posted comments on this change. Change subject: core: long method refactor ......................................................................
Patch Set 10: (2 comments) http://gerrit.ovirt.org/#/c/29588/10/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogDirector.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogDirector.java: Line 221: } Line 222: Line 223: private static void compose(StringBuilder builder, String key, String value) { Line 224: final char DELIMITER = ','; Line 225: builder.append(key).append('=').append(value).append(DELIMITER); > Imho , some style inconsistency. leftover from preexisting code. I still want to name coma as delimiter, since it more readable. Not feel the same for '=', but you're right it's inconsistent in style. So I've come up with "NAME_VALUE_SEPARATOR" for that character. I'm not satisfied with that, but it's best I was able to think of and it's worth of having delimiter named. DONE. Line 226: } Line 227: Line 228: private static String emptyGuidToEmptyString(Guid guid) { Line 229: return guid.equals(Guid.Empty) ? "" : guid.toString(); Line 232: private static String nullToEmptyString(Object obj) { Line 233: return toStringNullToDefault(obj, ""); Line 234: } Line 235: Line 236: private static String toStringNullToDefault(Object obj, String defaultValue) { > Please make sure we don't have this method somewhere else in the code, it d I moved it, but only to ObjectUtils since this class exists in common.utils while there's no class in uutils. So it make sence to place it in common.utils and move this class as a whole eventually. Is this ok? Line 237: return obj == null ? defaultValue : obj.toString(); Line 238: } Line 239: Line 240: static String resolveMessage(String message, AuditLogableBase logable) { -- To view, visit http://gerrit.ovirt.org/29588 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I832bc8c7e8bdfb2948f2137922e738fc6e702e59 Gerrit-PatchSet: 10 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Mucha <mmu...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@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: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: Yevgeny Zaspitsky <yzasp...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org 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
