2
\$\begingroup\$

I have a grid in which i have to display some/all fields of a Java Class, this grid displays the List of elements of that class type.

I have to sort this list provided:

  • Class type of the List item
  • Field name of the class to be sorted

to get field from a class by field name

private static <T> Field getField(Class<T> clazz,String fieldName){
    Field field = null;
    try {
        field = clazz.getDeclaredField(fieldName);
    }catch(java.lang.NoSuchFieldException e){
        e.printStackTrace();
    }
    return field;
}

and the comparator method using lambda, providing class type, field name to sort, and sort order

public static <T> Comparator<T> getGenericComparatorLambda(Class<T> clazz,
        String fieldName,boolean orderByAsc){

    //get field from class
    final Field field = getField(clazz, fieldName);
    field.setAccessible(true);

    //Lambda comparator
    Comparator<T> orderByComparator = (f1, f2) -> {

        Comparable compA = null;
        Comparable compB = null;

        // getting fields from object f1,f2
        try {
            compA =(Comparable) field.get(f1);
            compB =(Comparable) field.get(f2);
        } catch (IllegalAccessException e) {
            e.printStackTrace();
        }

        // handling null comparators
        if(compA == null)
            return -1;
        else if(compB == null)
            return 1;

        return compA.compareTo(compB);
    };

    //reverse comparator if order is descending
    if (orderByComparator != null && !orderByAsc)
        orderByComparator = orderByComparator.reversed();

    return orderByComparator;   
}

As the code consists of Lambda as well as generics of java, I want an overall analysis of the code, improvements where needed, how much is the performance penalty when using generics with long lists.

\$\endgroup\$

migrated from stackoverflow.com Jun 26 '17 at 11:50

This question came from our site for professional and enthusiast programmers.

  • \$\begingroup\$ You should not catch exception and continue with a null field. That is just exchanging the NoSuchFieldException with a NullPointerException. Further, the test orderByComparator != null at the end is obsolete, it is impossible that orderByComparator is null. And your comparator is inconsistent, if both compA and compB are null, you are returning -1. \$\endgroup\$ – Holger Jun 12 '17 at 11:59
2
\$\begingroup\$

If you want suggestions for improvement, here is one:

The following code:

private static <T> Field getField(Class<T> clazz,String fieldName){
    Field field = null;
    try {
        field = clazz.getDeclaredField(fieldName);
    }catch(java.lang.NoSuchFieldException e){
        e.printStackTrace();
    }
    return field;
}

is best rewritten as follows:

private static <T> Field getField(Class<T> clazz,String fieldName) {
    try {
        return clazz.getDeclaredField(fieldName);
    }catch(java.lang.NoSuchFieldException e){
        throw new RuntimeException(e);
    }
}

The following code:

//Lambda comparator
Comparator<T> orderByComparator = (f1, f2) -> {

    Comparable compA = null;
    Comparable compB = null;

    // getting fields from object f1,f2
    try {
        compA =(Comparable) field.get(f1);
        compB =(Comparable) field.get(f2);
    } catch (IllegalAccessException e) {
        e.printStackTrace();
    }

    // handling null comparators
    if(compA == null)
        return -1;
    else if(compB == null)
        return 1;

    return compA.compareTo(compB);
};

is best rewritten as follows:

Comparator<T> orderByComparator = (f1, f2) -> {
    Comparable compA, compB;
    try {
        compA = (Comparable) field.get(f1);
        compB = (Comparable) field.get(f2);
    } catch (IllegalAccessException e) {
        throw new RuntimeException(e);
    }
    return Objects.compare(compA, compB, Comparator.naturalOrder());
};

in short:

  • lose the useless comments; a comment is only useful if it says something that is not immediately obvious by looking at the code

  • do not initialize variables to nonsensical values, because then the compiler will not warn you when you forget to assign something meaningful to them

  • if you do not know what to do with an exception, do not print a stack trace, throw a runtime exception instead.

Generics and lambdas have no impact on performance.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.