SKU数比较

Question

声纳抱怨我有太多的return语句。如何才能很好地减少return语句的计数？

public int compare(Sku o1, Sku o2) {
    if (o1 == null) {
        return (o2 == null) ? 0 : 1;
    } else if (o2 == null) {
        return -1;
    }

    if (o1.getSkuNumber() == null) {
        return o2.getSkuNumber() == null ? 0 : 1;
    } else if (o2.getSkuNumber() == null) {
        return -1;
    }

    return o1.getSkuNumber().compareTo(o2.getSkuNumber());
}

Answer 1

由于您声明使用的是Java 8，所以您应该使用各种新的Comparator静态方法来提取相关字段，以便进行比较：

public int compare(Sku o1, Sku o2) {
    Comparator<Sku> c1 = Comparator.nullsLast(null);
    return c1.thenComparing(Sku::getSkuNumber,
            Comparator.nullsLast(Comparator.naturalOrder())).compare(o1, o2);
}

1. 当给Comparator.nullsLast(Comparator)一个null参数时，它认为两个非null参数是相等的.这是我们第一个比较国的基础。
2. 然后我们需要通过getSkuNumber()进行比较，因此我们将方法参考 Sku::getSkuNumber传递给Comparator.thenComparing(Function, Comparator)作为我们的第二个比较器。但是，由于SKU数字可能是null，所以我们再次使用嵌套(或三级) null-friendly比较器Comparator.nullsLast(Comparator)作为第二个参数。使用中的最后一个比较器就是Comparator.naturalOrder()，如果两个SKU数字都不是null，它将执行最后的直接整数比较。

编辑：@Simon提到了一个很好的观点，即由于您现有的compare(Sku, Sku)方法很可能是实现Comparable<Sku>的实现，您应该考虑将上面建议的Comparator设置为public static final字段，这样就不必每次都创建它，例如：

public static final Comparator<Sku> COMPARATOR = 
                        Comparator.nullsLast((Comparator<Sku>) null)
                            .thenComparing(Sku::getSkuNumber,
                                Comparator.nullsLast(Comparator.naturalOrder()));

Answer 2

您特别询问如何减少返回语句的数量，其中一个答案表示完全重写，这很可能是一个很好的选择。然而，为了解决减少返回声明数量的一般性问题，这里有一些想法可供考虑：

• 使用多个if条件或简化测试将类似的返回分组
• 存储返回结果，然后在结束时返回一次
• 如果可能的话，做一些早期的返回，以避免大量的测试和“困难”返回以后。

下面是一个修改后的变体，我使用一个简单的事实简化了测试，为了进行比较，您似乎认为o1 == null等于o1.getSkuNumber() == null，而我只测试了getSkuNumber()一次，而不是多次测试。如果getSkuNumber()很贵，那么避免多次使用它也是有意义的。

public int compare(Sku o1, Sku o2) {
    // Simplify into two variables to use when testing 
    Comparable o1num = o1 != null ? o1.getSkuNumber() : null;
    Comparable o2num = o2 != null ? o2.getSkuNumber() : null;
    Integer result = null;

    // Simplified test, with storage of return result
    if ( o1num == null && o2num == null) {
      result = 0;

    } else if (o1num == null && o2num != null) {
      result = 1;

    } else if (o1num != null && o2num == null) {
      result = -1;
    }

    // If return result is already set, return it, else
    // do the actual comparison
    return (result != null) ? result : o1num.compareTo(o2num);
}

对于这样一个简单的情况，它是否值得，这是一个品味问题，但至少简化o1与o1.getSkuNumber()的技巧是一个巧妙的技巧，我的第一个代码示例对我来说更温和一些。

您还可以选择一个可读性不强但较短的解决方案，如：

public int compare(Sku o1, Sku o2) {
    // Simplify into two variables to use when testing 
    Comparable o1num = o1 != null ? o1.getSkuNumber() : null;
    Comparable o2num = o2 != null ? o2.getSkuNumber() : null;

    // Early return if either (or both) is null
    if ( o1num == null || o2num == null) {
      return o1num == null ? (o2num == null ? 0 : 1) :  -1 ;
    } 

    // Do the actual comparison
    return o1num.compareTo(o2num);
}

最后一条简化了if的结构，因为它认识到所有的if's都依赖于一个或两个都为null。那么如果两者都是空返回0，或者如果只有一个返回1或-1取决于哪个返回为null。第二个返回语句作为两个都不为空，那么一般的比较是否都是空的。

编辑:改为Comparable，因为事实证明，getSkuNumber()不仅可以返回String。如注释中所建议的，进一步减少了最短的解决方案，但保留了(不必要的)括号级别，以保持一些可读性。:-)