4

我有几个Comparators - 一个代表Dates，一个代表小数，一个代表百分比，等等。

起初我的十进制比较器看起来像这样：

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);
  }
}

生活变得更好了。测试感觉更可靠。然而，我的代码审查员指出，“ nulls 呢？”

太好了，所以现在我必须重复上述NullPointerException内容或在方法主体前添加：

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

这个方法很大。最糟糕的是，我需要用其他三个比较不同类型的字符串的类重复这种模式，并且在解析时可能会引发其他三个异常。

我不是 Java 专家。有没有比复制和粘贴更清洁、更整洁的解决方案？只要记录在案，我是否应该以缺乏复杂性为代价换取正确性？

更新：有些人认为Comparator处理null值不是 ' 的工作。由于排序结果显示给用户，我确实希望对空值进行一致的排序。

4

12 回答 12

4

您正在实施一个Comparator<String>. String的方法，包括如果将 null 交给他们，则compareTo抛出 a ，所以你也应该这样做。NullPointerException同样，如果参数的类型阻止比较它们，则Comparator抛出 a 。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);
  }
}

通过提取一种方法来进行类型检查和转换，您几乎可以恢复原来的优雅。

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。在没有看到情况的情况下很难在那里提供细节，但是您可以在my 的一个版本上使用“Pull Up Method” ，使其成为其本身实现 java 的parseStringAsDouble共同祖先。NumericComparatorComparator

于 2009-11-05T20:03:04.450 回答
2

这个问题有很多主观答案。这是我自己的 0.02 美元。

首先，您所描述的问题是缺乏一流功能的语言的典型症状，这将使您能够简洁地描述这些模式。

其次，在我看来，如果其中一个不能被认为是双精度的表示，那么将两个字符串作为双精度进行比较应该是错误的。（对于 null 等也是如此。）因此，您应该允许异常传播！我预计这将是一个有争议的观点。

于 2009-11-05T19:00:57.483 回答
1

您可以创建一个实用方法来处理解析并在空值或解析异常的情况下返回某个值。

于 2009-11-05T18:58:04.160 回答
1

退后一步。那些Strings来自哪里？这是为了什么Comparator？你有一个Collection你Strings想要排序的吗？

于 2009-11-05T20:03:10.557 回答
1

tl;dr：从 JDK 获得指导。Double 比较器没有为非数字或空值定义。让人们给你有用的数据（双打、日期、恐龙等等）并为此编写你的比较器。

据我所知，这是一个用户输入验证的案例。例如，如果您从对话框中获取输入，则确保您拥有可解析的字符串的正确位置是 Double、Date 或输入处理程序中的任何内容。确保它在用户可以使用标签之前很好，点击“Okay”或同等的。

这就是我认为的原因：

第一个问题：如果字符串不能解析为数字，我认为你试图在错误的地方解决问题。比如说，我尝试"1.0"比较"Two". 第二个显然不能作为 Double 解析，但它比第一个少吗？还是更大。我认为用户应该在询问您哪个更大之前将他们的字符串转换为双精度（例如，您可以使用 Double.compareTo 轻松回答）。

第二个问题：如果字符串是"1.0"and null，哪个更大？JDK 源不处理 Comparator 中的 NullPointerExceptions：如果给它一个 null，自动装箱将失败。

最糟糕的是，我需要用其他三个比较不同类型的字符串的类重复这种模式，并且在解析时可能会引发其他三个异常。

正是为什么我会争辩说解析应该发生在您的 Comparator 之外，并在它到达您的代码之前处理异常处理。

于 2009-11-05T20:08:00.953 回答
1

试试这个：

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

于 2009-11-05T20:36:31.043 回答
1

以下是我改进比较器的方法：

首先，提取一个转换值的方法。它被重复，多次尝试......捕获总是丑陋 -> 最好尽可能少地拥有它们。

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:s（或者像其他人喜欢说的那样，“实现比较器”）：

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;
          }
     }
}

...是的，这是一个分支嵌套的三元组。如果有人抱怨它，请说出这里其他人一直在说的话：处理空值不是 Comparator 的工作。

于 2009-11-05T20:38:31.623 回答
1

似乎这里有两个问题混合在一起，也许应该分解成单独的组件。考虑以下：

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。

于 2009-11-05T22:36:20.703 回答
0

如果您能够更改签名，我建议您编写该方法，以便它可以接受任何受支持的对象。

  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");
  }

您甚至可以递归地定义它，将字符串转换为 Doubles，然后返回使用这些对象调用自身的结果。

于 2009-11-05T19:49:03.470 回答
0

恕我直言，您应该首先创建一个从字符串返回 Double 的方法，嵌入 null 并解析失败情况（但您必须定义在这种情况下要做什么：抛出异常？返回默认值？？）。

然后你的比较器只需要比较获得的 Double 实例。

换句话说，重构...

但是我仍然想知道为什么您需要比较字符串，尽管期望它们代表双打。我的意思是，是什么阻止您在实际使用此比较器的代码中操作双精度值？

于 2009-11-05T19:56:19.977 回答
0

根据您的需要和 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;
    }

}

我将该方法提取为一个抽象类，可以在其他情况下重用（尽管类名很烂）。

干杯。

于 2009-11-06T02:21:13.803 回答
-3

所以我改进了 compare()...

确定你做到了。

首先，Comparator 接口没有指定空值会发生什么。如果您的 null 检查 if 语句适用于您的用例，那很好，但一般的解决方案是抛出一个 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;  
 }
}

似乎您可能会发现将test重命名为t1并将retVal重命名为q更清晰。

至于重复模式……嗯。您也许可以使用带有反射的泛型来调用适当的 parseX方法。似乎那不值得。

于 2009-11-05T20:05:02.120 回答