Moti Asayag has posted comments on this change.

Change subject: core: long method refactor
......................................................................


Patch Set 17:

(2 comments)

https://gerrit.ovirt.org/#/c/29588/17//COMMIT_MSG
Commit Message:

Line 10: 
Line 11: this code is more readable with each pair on one line. Can be better
Line 12: if refactored into class, if one doing CR is ok with that, I can do
Line 13: it.
Line 14: Comment: isn't it an error, ending with coma?
it seems that the last comma should be removed. however i wouldn't call it an 
error, rather a weird way to construct object id :-)

feel free to remove it in your next patch.
Line 15: 
Line 16: Change-Id: I832bc8c7e8bdfb2948f2137922e738fc6e702e59


https://gerrit.ovirt.org/#/c/29588/17/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 226:     private static String nullToEmptyString(Object obj) {
Line 227:         return toStringNullToDefault(obj, "");
Line 228:     }
Line 229: 
Line 230:     private static String toStringNullToDefault(Object obj, String 
defaultValue) {
i don't see the point in this method since only the method above it uses it.

in addition, this functionality already provided by 
java.util.Objects.toString(Object o, String nullDefault):

    /**
     * Returns the result of calling {@code toString} on the first
     * argument if the first argument is not {@code null} and returns
     * the second argument otherwise.
     *
     * @param o an object
     * @param nullDefault string to return if the first argument is
     *        {@code null}
     * @return the result of calling {@code toString} on the first
     * argument if it is not {@code null} and the second argument
     * otherwise.
     * @see Objects#toString(Object)
     */
    public static String toString(Object o, String nullDefault) {
        return (o != null) ? o.toString() : nullDefault;
    }
Line 231:         return obj == null ? defaultValue : obj.toString();
Line 232:     }
Line 233: 
Line 234:     static String resolveMessage(String message, AuditLogableBase 
logable) {


-- 
To view, visit https://gerrit.ovirt.org/29588
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I832bc8c7e8bdfb2948f2137922e738fc6e702e59
Gerrit-PatchSet: 17
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Martin Mucha <mmu...@redhat.com>
Gerrit-Reviewer: Eli Mesika <emes...@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 <wallaroo1...@gmail.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

Reply via email to