Vojtech Szocs has posted comments on this change. Change subject: webadmin: Implement sorting for RxTxRateColumn ......................................................................
Patch Set 11: (1 comment) 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(); > TextColumnWithTooltip's comparator is only instantiated if makeSortable() i OK, I see your point :-) I am OK with current approach. I always tried to avoid overriding methods without delegating to super implementation, but in some cases it makes sense. However, note that if TextColumnWithTooltip had code like this: public void makeSortable() { makeSortable(getDefaultComparator()); } // LexoNumericComparator instance created only when this method is called protected Comparator<T> getDefaultComparator() { return new Comparator<T>() { private LexoNumericComparator lexoNumeric = new LexoNumericComparator(); @Override public int compare(T arg0, T arg1) { return lexoNumeric.compare(getValue(arg0), getValue(arg1)); } } } then it would save you the effort of duplicating some code, instead of: private LexoNumericComparator lexoNumeric = new LexoNumericComparator(); ... return lexoNumeric.compare(text1, text2); you could just do: ... 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); -- 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 <[email protected]> Gerrit-Reviewer: Alexander Wels <[email protected]> Gerrit-Reviewer: Alona Kaplan <[email protected]> Gerrit-Reviewer: Gilad Chaplik <[email protected]> Gerrit-Reviewer: Lior Vernia <[email protected]> Gerrit-Reviewer: Vojtech Szocs <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
