Change in ovirt-engine[master]: webamdin: Standardize AbstractVmPopupWidget#DataCenterWithCl...

Code Review gerrit at ovirt.org
Thu Jun 22 14:54:20 UTC 2017


>From Allon Mureinik <amureini at redhat.com>:

Allon Mureinik has submitted this change and it was merged.

Change subject: webamdin: Standardize AbstractVmPopupWidget#DataCenterWithClusterComparator
......................................................................


webamdin: Standardize AbstractVmPopupWidget#DataCenterWithClusterComparator

DataCenterWithClusterComparator is a boilerplate implementation for
sorting DataCenterWithCluster objects by the DC name and then by the
cluster name.

This patch standardizes the implementation by removing the
boilerplate code and replacing it with a chained
Comparator.comapring(...).thenComapring call.

The original code seems to have several slight bugs which do not seem
to be the intended behavior based on the class' comment ("Comparator
that sorts on data center name first, and then cluster name. Ignoring
case."), and may lead to inconsistent sorting:
- Two objects with a null DC name will be considered equivalent,
  regardless of the cluster name. This seems inconsistent with the
  behavior of sorting two objects with the same DC name by the
  cluster name.
- Moreover, for two objects have a non-null DC name, if they have
  equal (*case sensitive*) DC names, they are sorted by the cluster
  name, but if the DC names are not equal, they are sorted by it in a
  *case insensitive* order.

The revised code takes a cleaner, more obvious approach:
- Sort by the DC names, in a case insensitive order, with the nulls
  last.
- Break ties by sorting by cluster names, in a case insensitive
  order, again with nulls last.

Change-Id: I6e5a06a8af094b3693228c29ed87182b4ae3183a
Signed-off-by: Allon Mureinik <amureini at redhat.com>
---
M frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/uicommon/popup/AbstractVmPopupWidget.java
1 file changed, 5 insertions(+), 28 deletions(-)

Approvals:
  Jenkins CI: Passed CI tests
  Allon Mureinik: Verified
  Vojtech Szocs: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I6e5a06a8af094b3693228c29ed87182b4ae3183a
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Alexander Wels <awels at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Greg Sheremeta <gshereme at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Tomas Jelinek <tjelinek at redhat.com>
Gerrit-Reviewer: Vojtech Szocs <vszocs at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>


More information about the Engine-commits mailing list