Vojtech Szocs has posted comments on this change. Change subject: webadmin: System tree refresh ......................................................................
Patch Set 3: (3 comments) http://gerrit.ovirt.org/#/c/26161/3/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/SystemTreeItemModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/SystemTreeItemModel.java: Line 131: } Line 132: if (result) { Line 133: if ((otherModel.getEntity() == null && getEntity() == null) Line 134: || (otherModel.getEntity() != null && otherModel.getEntity().equals(getEntity()) Line 135: && otherModel.getTitle().equals(getTitle()))) { I know that it's unlikely for the title attribute to be null, but to honor the contract of equals method, I suggest to perform null check here. Line 136: result = true; Line 137: } else { Line 138: result = false; Line 139: } Line 132: if (result) { Line 133: if ((otherModel.getEntity() == null && getEntity() == null) Line 134: || (otherModel.getEntity() != null && otherModel.getEntity().equals(getEntity()) Line 135: && otherModel.getTitle().equals(getTitle()))) { Line 136: result = true; Hm, this seems a bit strange to me, as result is already true here. Wouldn't multiple return statements in this method make it more readable, instead of enforcing single return value? Line 137: } else { Line 138: result = false; Line 139: } Line 140: } Line 145: public int hashCode() { Line 146: final int prime = 31; Line 147: int result = 1; Line 148: result = prime * result + getTitle().hashCode(); Line 149: result = prime * result + super.hashCode(); I'd suggest to consider conventional hashCode implementation that is consistent with equals method - taking both entity and title into account, both of which can be null: final int prime = 31; int result = super.hashCode(); result = prime * result + ((getEntity() == null) ? 0 : getEntity().hashCode()); result = prime * result + ((getTitle() == null) ? 0 : getTitle().hashCode()); return result; (If two objects are equal according to the equals method, then calling the hashCode method on each of the two objects must produce the same integer result.) Line 150: return result; Line 151: } Line 152: -- To view, visit http://gerrit.ovirt.org/26161 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I062163b1f6d2e5fcfd1539b69a3d0d2cee674654 Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alexander Wels <aw...@redhat.com> Gerrit-Reviewer: Alexander Wels <aw...@redhat.com> Gerrit-Reviewer: Einav Cohen <eco...@redhat.com> Gerrit-Reviewer: Greg Sheremeta <gsher...@redhat.com> Gerrit-Reviewer: Lior Vernia <lver...@redhat.com> Gerrit-Reviewer: Vojtech Szocs <vsz...@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
