Greg Padgett has posted comments on this change. Change subject: WIP core: fix businessentity hashCode/equals errors ......................................................................
Patch Set 3: (2 inline comments) .................................................... File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/QuotaStorage.java Line 139: int result = 1; Line 140: result = prime * result + ((quotaId == null) ? 0 : quotaId.hashCode()); Line 141: result = prime * result + ((quotaStorageId == null) ? 0 : quotaStorageId.hashCode()); Line 142: result = prime * result + ((storageId == null) ? 0 : storageId.hashCode()); Line 143: result = prime * result + ((storageLimitGigaByteUsage == null) ? 0 : storageLimitGigaByteUsage.hashCode()); I see 2 drawbacks to not comparing the entire object, mainly from having to change equals to match hashCode behavior: a) it wouldn't be orthogonal with behavior of other business entities (thus caution would be required, maybe more error-prone), and b) you lose any easy way to compare entire objects for the small gain of not having to reference the id field. Maybe it wouldn't ever matter... Line 144: result = prime * result + ((storageLimitGigaByte == null) ? 0 : storageLimitGigaByte.hashCode()); Line 145: return result; Line 146: } Line 147: Line 140: result = prime * result + ((quotaId == null) ? 0 : quotaId.hashCode()); Line 141: result = prime * result + ((quotaStorageId == null) ? 0 : quotaStorageId.hashCode()); Line 142: result = prime * result + ((storageId == null) ? 0 : storageId.hashCode()); Line 143: result = prime * result + ((storageLimitGigaByteUsage == null) ? 0 : storageLimitGigaByteUsage.hashCode()); Line 144: result = prime * result + ((storageLimitGigaByte == null) ? 0 : storageLimitGigaByte.hashCode()); Indeed it is, from both hashCode and equals. Line 145: return result; Line 146: } Line 147: Line 148: @Override -- To view, visit http://gerrit.ovirt.org/12539 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic83c16ab9bbbb95f45bfcd91920f8137890e63d8 Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Greg Padgett <gpadg...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com> Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com> Gerrit-Reviewer: Greg Padgett <gpadg...@redhat.com> Gerrit-Reviewer: Laszlo Hornyak <lhorn...@redhat.com> Gerrit-Reviewer: Noam Slomianko <nslom...@redhat.com> Gerrit-Reviewer: ofri masad <oma...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches