I was advised not to use reflection here ... Why not?

Question

This code is used to sort the list. The list may contain thousands of elements, but less than 10 thousand.

protected <E> int compareFields(E o1, E o2, String fieldName){ try { Comparable o1Data = (Comparable) o1.getClass().getMethod(fieldName).invoke(o1); Comparable o2Data = (Comparable) o2.getClass().getMethod(fieldName).invoke(o2); return o1Data == null ? o2Data == null ? 0 : 1 : o2Data == null ? -1 : o1Data.compareTo(o2Data); } catch(Exception e) { throw new RuntimeException(e); } } 

I was advised

"Please do not use reflection for such things! Either put a method using a suitable Comparator, or a method to extract the corresponding property (can be computed in a way that is not supported by the original type), or both."

An example of a better way to do this would be nice.

Context: I have many screens with data tables. Each of them is created from the list. Each data table should be sorted by each of the six columns. Columns are Date or String.

Answer 1

Using reflection here will be potentially much slower as you add the number of stack frames to each comparison using getClass , getMethod and invoke rather than using your own object comparison method.

Ideally, you should write a method to avoid using object in the signature. The “appropriate comparator” should at least be strictly tied to the type of objects (which, in your opinion, is one and the same). If you must have a dynamic field mapping (as it appears), then at least the reflection can be encapsulated in this comparator.

If you are going to call it a thousand times, it would be better to first bind the comparator to the field that you are sorting. This way you only call getMethod first, and not once for each individual comparison.

Answer 2

It's hard to give a good example without context, so now here is a short list of why this is not a good idea:

• The provided field does not guarantee compatibility (I'm not sure why the code here should catch this exception and rename it).
• What if the type of objects is not intended to be compared in this way? (This is too common a method name to know how it should be used).
• He did not score much. Providing the field name as a string means that you have to change your code everywhere when the property name changes, and it will be difficult to track where you need to make these changes.
• Reflection is potentially slower than if it were implemented in a strongly typed manner.

Answer 3

Other answers are well described, why reflection is not recommended. I want to add an example using a more traditional solution.

Instead of specifying a field that is used to compare two objects, you should take an instance of Comparator as an argument. Thus, a client that uses this method can specify how to compare two objects.

 protected <E> int compareFields(E o1, E o2, Comparator<E> comparator) { return comparator.compare(o1, o2); } 

And an example call to this function would look like this:

 MyClass a = ...; MyClass b = ...; Comparator<MyClass> intFieldComparator = new Comparator<MyClass> { public int compare(MyClass o1, MyClass o2) { int field1 = o1.getIntField(); int field2 = o2.getIntField(); return field2 - field1; } }; compareFields(a, b, intFieldComparator); 

You can define different comparators if you want to compare objects using several fields.