Improve custom comparator code quality

Question

Any better way to write this comparator?

Comparator<JSONObject> comparator = (op1,op2) -> {
         Integer id1 = Integer.valueOf(String.valueOf(op1.get(field)));
         Integer id2 = Integer.valueOf(String.valueOf(op2.get(field)));
         return id2.compareTo(id1);
         };

Is it possible to avoid the two valueOf?

Answer 1

Using Comparator.comparingInt

You should be able to do the following:

Comparator<JSONObject> comparator = 
    Comparator.comparingInt(obj -> Integer.valueOf(String.valueOf(obj.get(field))))
    .reversed();

getInt

Check the API of the JSONObject to see if there is an getInt method or similar on it, then you should be able to use that directly instead of wrapping in valueOf calls, such as:

Comparator<JSONObject> comparator = Comparator.<JSONObject>comparingInt(obj -> obj.getInt(field))
    .reversed();

Return type of .get

Alternatively, you should investigate what type your obj.get(field) returns. If you are lucky, it can be as simply as a typecast:

Comparator<JSONObject> comparator = 
    Comparator.<JSONObject>comparingInt(obj -> (Integer) obj.get(field))
    .reversed();

Return type is String

If you are always getting Strings as a result from obj.get, then you should be able to use Integer.valueOf directly, instead of also calling String.valueOf

Comparator<JSONObject> comparator = 
    Comparator.<JSONObject>comparingInt(obj -> Integer.valueOf(obj.get(field)))
    .reversed();

Depending on the API, you still might need to typecase to String

Comparator<JSONObject> comparator = 
    Comparator.<JSONObject>comparingInt(obj -> Integer.valueOf((String) obj.get(field)))
    .reversed();

Answer 2

Is it possible to avoid the two valueOf?

This depends on whether or not know the type of field your are trying to get, ie: is it an int or a string or either

if it is a guaranteed int you can simply do this

Comparator<JSONObject> comparator = (op1,op2) -> {
     return Integer.valueOf(op1.get(field)).compareTo(Integer.valueOf(op2.get(field));
};

for ascending order

if value is a string you can do this

Comparator<JSONObject> comparator = (op1,op2) -> {
     return Integer.parseInt((String)op1.get(field)) - Integer.parseInt((String)op2.get(field));
};