Как я могу улучшить этот компаратор?

Question

У меня есть несколько Comparator с - один для Date с, один для знаков после запятой, один за проценты и т.д.

В первом моем десятичном компараторе выглядел следующим образом:

class NumericComparator implements Comparator<String> { 

    @Override 
    public int compare(String s1, String s2) { 
    final Double i1 = Double.parseDouble(s1); 
    final Double i2 = Double.parseDouble(s2); 
    return i1.compareTo(i2); 
    } 

} 

Жизнь была проста , Конечно, это не относится к случаю, когда строки не обрабатываются. Таким образом, я улучшил compare():

class NumericComparator implements Comparator<String> { 

    @Override 
    public int compare(String s1, String s2) { 
    final Double i1; 
    final Double i2; 

    try { 
     i1 = Double.parseDouble(s1); 
    } catch (NumberFormatException e) { 
     try { 
     i2 = Double.parseDouble(s2); 
     } catch (NumberFormatException e2) { 
     return 0; 
     } 
     return -1; 
    } 
    try { 
     i2 = Double.parseDouble(s2); 
    } catch (NumberFormatException e) { 
     return 1; 
    } 

    return i1.compareTo(i2); 
    } 
} 

Жизнь была лучше. Испытания стали более твердыми. Однако мой обозреватель моего кода отметил: «А как насчет null?»

Великий, так что теперь я должен повторить выше NullPointerException или предварять тело метода с:

if (s1 == null) { 
    if (s2 == null) { 
    return 0; 
    } else { 
    return -1; 
    } 
} else if (s2 == null) { 
    return 1; 
} 

Этот метод огромен. Хуже всего, мне нужно повторить этот шаблон с тремя другими классами, которые сравнивают различные типы строк и могут поднять три других исключения при разборе.

Я не эксперт по Java. Есть ли более чистый, более чистый раствор, чем - gasp - копирование и склеивание? Должен ли я торговать правильностью из-за отсутствия сложности, если она документально подтверждена?

---

Update: Некоторые предположили, что это не задание Comparator «s для обработки null значений. Поскольку результаты сортировки отображаются пользователям, я действительно хочу, чтобы нули сортировались последовательно.

Answer 1

Вот как я бы улучшить компаратор:

Во-первых, exctract способ преобразования значения. Это повторяется, несколько попыток ... уловы всегда уродливы -> лучше иметь как можно меньше из них.

private Double getDouble(String number) { 
try { 
    return Double.parseDouble(number); 
} catch(NumberFormatException e) { 
    return null; 
} 
} 

Далее, запишите простые правила, чтобы показать, как вы хотите, чтобы поток компаратора был.

if i1==null && i2!=null return -1 
if i1==null && i2==null return 0 
if i1!=null && i2==null return 1 
if i1!=null && i2!=null return comparison 

Наконец делать ужасное запутывание к фактическому компаратору, чтобы поднять несколько WTF: с в обзоре коды (или как другие любят говорить, «Реализовать компаратор»):

class NumericComparator implements Comparator<String> { 

    public int compare(String s1, String s2) { 
     final Double i1 = getDouble(s1); 
     final Double i2 = getDouble(s2); 

     return (i1 == null) ? (i2 == null) ? 0 : -1 : (i2 == null) ? 1 : i1.compareTo(i2); 
    } 
    private Double getDouble(String number) { 
      try { 
       return Double.parseDouble(number); 
      } catch(NumberFormatException e) { 
       return null; 
      } 
    } 
} 

... да, это ветвистая вложенная тройка. Если кто-то жалуется на это, скажите, что говорили другие: Обработка нулей - это не работа компаратора.

Answer 2

Вы можете создать метод утилиты, который обрабатывает разбор и возвращает определенное значение в случае исключений NULL или синтаксического анализа.

Answer 3

Есть много субъективных ответов на этот вопрос. Вот мои собственные $ 0,02.

Во-первых, проблема, которую вы описываете, - это канонический симптом языка, в котором отсутствуют первоклассные функции, которые позволят вам кратко описать эти шаблоны.

Во-вторых, на мой взгляд, это должно быть быть ошибкой сравнивать две строки как пары, если один из них нельзя считать представлением двойника. (То же самое верно для нулей и т. Д.). Поэтому вы должны разрешать распространение исключений! Это будет спорным мнение, я ожидаю.

Answer 4

Если вы в состоянии изменить подпись, я предлагаю вам написать метод, чтобы он мог принимать любой поддерживаемый объект.

public int compare(Object o1, Object o2) throws ClassNotFoundException { 
     String[] supportedClasses = {"String", "Double", "Integer"}; 
     String j = "java.lang."; 
     for(String s : supportedClasses){ 
      if(Class.forName(j+s).isInstance(o1) && Class.forName(j+s).isInstance(o1)){ 
       // compare apples to apples 
       return ((Comparable)o1).compareTo((Comparable)o2); 
      } 
     } 
     throw new ClassNotFoundException("Not a supported Class"); 
    } 

Можно даже определить рекурсивно, где вы приводите ваши строки в парном разряде, а затем возвращает результат вызова себя с этими объектами.

Answer 5

IMHO вы должны сначала создать метод, который возвращает Double из String, встраивая случаи сбоя с ошибкой и синтаксического анализа (но вы должны определить, что делать в таких случаях: вызывать исключение? Возвращать значение по умолчанию?).

Тогда ваш компаратор просто должен сравнить полученные двойные экземпляры.

Другими словами, рефакторинг ...

Но я до сих пор удивляюсь, почему вам нужно сравнивать строки, хотя ожидали они представляют двойников. Я имею в виду, что мешает вам манипулировать двойниками в коде, который фактически использовал бы этот компаратор?

Answer 6

Вы используете Comparator<String>. String, в том числе compareTo, введите NullPointerException, если ему передан нуль, поэтому вам тоже нужно. Аналогично, Comparator выбрасывает ClassCastException, если типы аргументов не позволяют сравнивать их. Я бы рекомендовал вам реализовать эти унаследованные поведения.

class NumericComparator implements Comparator<String> { 

    public int compare(String s1, String s2) { 
    final Double i1; 
    final Double i2; 
    if(s1 == null) 
    { 
     throw new NullPointerException("s1 is null"); // String behavior 
    } 
    try { 
     i1 = Double.parseDouble(s1) 
    } catch (NumberFormatException e) { 
     throw new ClassCastException("s1 incorrect format"); // Comparator behavior 
    } 

    if(s2 == null) 
    { 
     throw new NullPointerException("s2 is null"); // String behavior 
    } 
    try { 
     i2 = Double.parseDouble(s1) 
    } catch (NumberFormatException e) { 
     throw new ClassCastException("s2 incorrect format"); // Comparator behavior 
    } 
    return i1.compareTo(i2); 
    } 
} 

Вы можете почти восстановить первоначальную элегантность путем extracting a method сделать проверку типов и преобразования.

class NumericComparator implements Comparator<String> { 

    public int compare(String s1, String s2) { 
    final Double i1; 
    final Double i2; 

    i1 = parseStringAsDouble(s1, "s1"); 
    i2 = parseStringAsDouble(s2, "s2"); 
    return i1.compareTo(i2); 
    } 

    private double parseStringAsDouble(String s, String name) { 

    Double i; 
    if(s == null) { 
     throw new NullPointerException(name + " is null"); // String behavior 
    } 
    try { 
     i = Double.parseDouble(s1) 
    } catch (NumberFormatException e) { 
     throw new ClassCastException(name + " incorrect format"); // Comparator behavior 
    } 
    return i; 
    } 
} 

Если вы не используете сообщения об исключении, вы можете потерять параметр «имя». Я уверен, что вы можете потерять лишнюю строку здесь или слово там, применив небольшие трюки.

Вы говорите, что вам нужно repeat this pattern with three other classes which compare different types of strings and could raise three other exceptions. Трудно предлагать специфику там, не видя ситуации, но вы можете использовать "Pull Up Method" в версии моего parseStringAsDouble в общем предке NumericComparator, который сам реализует java's Comparator.

Answer 7

Сделайте шаг назад. Откуда берутся эти Strings? Для чего это Comparator для использования? У вас есть CollectionStrings, который вы хотели бы отсортировать или так?

Answer 8

Так я улучшил сравнить() ...

, что вы сделали.

Во-первых, интерфейс компаратора не указывает, что происходит с нулями. если ваша нулевая проверка, если оператор работает для вашего варианта использования, это здорово, но общее решение бросает npe.

что касается чистых ... почему окончательный? почему все ловушки/броски? зачем использовать compareTo для примитивной оболочки?

class NumericComparator implements Comparator<String> { 
public int compare(String s1, String s2) throws NullPointerException, NumberFormatException { 

    double test = Double.parseDouble(s1) - Double.parseDouble(s2); 

    int retVal = 0; 
    if (test < 0) retVal = -1; 
    else if (test > 0) retVal = 1; 

    return retVal; 
} 
} 

кажется, что вы могли бы найти его более четким, переименовав тест к t1 и RetVal к д.

что касается повторяющегося рисунка ... а. вы могли бы использовать дженерики с отражением для вызова соответствующих методов parseX. похоже, это не стоило того.

Answer 9

tl; dr: Взять руководство от JDK. Двойной компаратор не определен ни для числа, ни для нулей. Заставьте людей дать вам полезные данные (Doubles, Dates, Dinosaurs, что угодно) и напишите для этого своих компараторов.

Насколько я могу судить, это случай проверки ввода пользователем. Например, если вы вносите ввод из диалогового окна, правильно место, чтобы убедиться, что у вас есть синтаксическая строка, которая представляет собой Double, Date или что-то еще в обработчике ввода. Удостоверьтесь, что это хорошо, прежде чем пользователь может уйти, нажмите «Хорошо» или эквивалент.

Вот почему я думаю, что это:

Первый вопрос: если строки не распознаваем как числа, я думаю, вы пытаетесь решить проблему в неправильном месте. Скажем, например, я пытаюсь сравнить "1.0" с "Two". Второй, очевидно, не поддается анализу, как двойной, но он меньше первого? Или это больше. Я хотел бы утверждать, что пользователям нужно будет превратить свои строки в парные, прежде чем они спросят, что больше (на что вы можете легко ответить с помощью Double.compareTo, например).

Второй вопрос: если строки "1.0" и null, что больше? Источник JDK не обрабатывает NullPointerExceptions в компараторе: если вы дадите ему null, автобоксинг завершится неудачей.

Хуже всего, мне нужно повторить этот шаблон с тремя другими классами которые сравнивают различные типы строк и может поднять три других исключения при разборе.

Именно поэтому я хотел бы утверждать, что синтаксический разбор должен происходить за пределами вашего компаратора с обработкой исключительных ситуаций, прежде чем он поступит в ваш код.

Answer 10

Попробуйте это:

import com.google.common.base.Function; 
import com.google.common.collect.Ordering; 

Ordering.nullsFirst().onResultOf(
    new Function<String, Double>() { 
     public Double apply(String s) { 
     try { 
     return Double.parseDouble(s); 
     } catch (NumberFormatException e) { 
     return null; 
     } 
    }) 

Единственная проблема, если это вы считаете это, что, является то, что пустые строки и другие не распознаваем Струны все будут перемешаны. Это, вероятно, не имеет большого значения, учитывая преимущества - это дает вам компаратор, который гарантированно будет правильным, тогда как с компаратором с ручным кодированием, даже относительно простым, удивительно, насколько легко совершить тонкую ошибку, которая ломается транзитивность или, ммм, антисимметричность.

http://google-collections.googlecode.com

Answer 11

Кажется, что здесь есть две проблемы, и их можно разбить на отдельные компоненты. Рассмотрим следующий пример:

public class ParsingComparator implements Comparator<String> { 
    private Parser parser; 

    public int compare(String s1, String s2) { 
    Object c1 = parser.parse(s1); 
    Object c2 = parser.parse(s2); 
    new CompareToBuilder().append(c1, c2).toComparison(); 
    } 
} 

интерфейс Parser будет иметь реализации для чисел, дат и т.д. Вы могли бы потенциально использовать класс java.text.Format для интерфейса Parser. Если вы не хотите использовать commons-lang, вы можете заменить использование CompareToBuilder некоторой логикой для обработки нулей и использовать Comparable вместо Object для c1 и c2.

Answer 12

в соответствии с вашими потребностями и пост Ewan, я думаю, что есть способ извлечь структуру, что вы можете повторно использовать:

class NumericComparator implements Comparator<String> { 
    private SafeAdaptor<Double> doubleAdaptor = new SafeAdaptor<Double>(){ 
     public Double parse(String s) { 
      return Double.parseDouble(s); 
     } 
    }; 
    public int compare(String s1, String s2) { 
     final Double i1 =doubleAdaptor.getValue(s1, "s1"); 
     final Double i2 = doubleAdaptor.getValue(s2, "s2"); 
     return i1.compareTo(i2); 
    } 
} 

abstract class SafeAdaptor<T>{ 
    public abstract T parse(String s); 
    public T getValue(String str, String name) { 
     T i; 
     if (str == null) { 
      throw new NullPointerException(name + " is null"); // String 
     } 
     try { 
      i = parse(str); 
     } catch (NumberFormatException e) { 
      throw new ClassCastException(name + " incorrect format"); // Comparator 
     } 
     return i; 
    } 

} 

извлечь метод как абстрактный класс, который можно повторно использовать в других случаях (хотя имя класса сосать).

ура.