Vojtech Szocs has posted comments on this change. Change subject: webadmin: Implement sorting for RxTxRateColumn ......................................................................
Patch Set 11: (2 comments) http://gerrit.ovirt.org/#/c/28757/11/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/column/RxTxRateColumn.java File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/column/RxTxRateColumn.java: Line 37: @Override Line 38: public void makeSortable() { Line 39: makeSortable(new Comparator<T>() { Line 40: Line 41: private LexoNumericComparator lexoNumeric = new LexoNumericComparator(); Here, we override TextColumnWithTooltip.makeSortable() which already works with LexoNumericComparator. If TextColumnWithTooltip exposed its LexoNumericComparator like this: public abstract class TextColumnWithTooltip<T> ... { private final LexoNumericComparator lexoNumeric = new LexoNumericComparator(); ... public void makeSortable() { // no anonymous-class-specific comparator, just use lexoNumeric declared above } protected LexoNumericComparator getDefaultComparator() { return lexoNumeric; } } then we could remove "lexoNumeric" here (RxTxRateColumn) and instead of this: return lexoNumeric.compare(text1, text2); we could just do this: return getDefaultComparator().compare(text1, text2); Line 42: Line 43: @Override Line 44: public int compare(T o1, T o2) { Line 45: String text1 = getValue(o1); Line 45: String text1 = getValue(o1); Line 46: String text2 = getValue(o2); Line 47: if (text1.equals(text2)) { Line 48: return 0; Line 49: } else if (RxTxRateRenderer.NO_VALUE.equals(text1) || RxTxRateRenderer.NO_VALUE.equals(text2)) { Alternative approach for consideration: ... if (renderer.isNoValue(text1) || renderer.isNoValue(text2)) { return renderer.isNoValue(text1) ? -1 : 1; } ... That way, RxTxRateRenderer.NO_VALUE and similar constants could be considered as implementation detail. Line 50: return RxTxRateRenderer.NO_VALUE.equals(text1) ? -1 : 1; Line 51: } else if (RxTxRateRenderer.ZERO_VALUE.equals(text1) || RxTxRateRenderer.ZERO_VALUE.equals(text2)) { Line 52: return RxTxRateRenderer.ZERO_VALUE.equals(text1) ? -1 : 1; Line 53: } else if (RxTxRateRenderer.SMALL_VALUE.equals(text1) || RxTxRateRenderer.SMALL_VALUE.equals(text2)) { -- To view, visit http://gerrit.ovirt.org/28757 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia0a9b91350e774639a7879e26456f6e8cc15ba3d Gerrit-PatchSet: 11 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Lior Vernia <lver...@redhat.com> Gerrit-Reviewer: Alexander Wels <aw...@redhat.com> Gerrit-Reviewer: Alona Kaplan <alkap...@redhat.com> Gerrit-Reviewer: Gilad Chaplik <gchap...@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
