Lior Vernia 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 TextColumnWithTooltip's comparator is only instantiated if makeSortable() is called. I don't see a benefit in having it instantiated and then re-using it - I think it's perfectly okay for RxTxRateColumn to override the entire sorting behavior without referring to any superclass implementation. 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: Done 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
