在 Optional 上使用 get() 是不好的做法吗?

Is it bad practise to use get() on an Optional?

我想对前一千个质数求和。当我尝试这个时...

System.out.println(getFirstThousandPrimes().stream()
                                           .reduce(Integer::sum)
                                           .get()
    );

IntelliJ 建议我检查 isPresent(),但这可能吗?

另一种选择是使用 .orElse(-1),但我不想 return 任何东西。我应该抛出异常吗?

通过将流减少为参数,流有可能为空,因此引入了 Optional 来处理这种情况。还有一种 reduce() 方法接受身份参数,因此 reduce Optionalreturned 因为如果是空流,身份将被返回。

但专注于您的任务,我建议使用自定义收集器来划分质数和非质数,然后对前 n 个数求和。

前段时间我实现了质数和非质数的收集器,实现如下所示:

public class PrimeNumberCollector implements Collector<Integer,
    Map<Boolean, List<Integer>>,
    Map<Boolean, List<Integer>>> {

@Override
public Supplier<Map<Boolean, List<Integer>>> supplier() {
    return () -> new HashMap<Boolean, List<Integer>>() {{
        put(Boolean.TRUE, new ArrayList<>());
        put(Boolean.FALSE, new ArrayList<>());
    }};
}

@Override
public BiConsumer<Map<Boolean, List<Integer>>, Integer> accumulator() {
    return (Map<Boolean, List<Integer>> acc, Integer candidate) -> acc.get(isPrime(candidate))
            .add(candidate);
}

@Override
public BinaryOperator<Map<Boolean, List<Integer>>> combiner() {
    return (Map<Boolean, List<Integer>> firstMap, Map<Boolean, List<Integer>> secondMap) -> {
        firstMap.get(Boolean.TRUE).addAll(secondMap.get(Boolean.TRUE));
        firstMap.get(Boolean.FALSE).addAll(secondMap.get(Boolean.FALSE));

        return firstMap;
    };
}

@Override
public Function<Map<Boolean, List<Integer>>, Map<Boolean, List<Integer>>> finisher() {
    return Function.identity();
}

@Override
public Set<Characteristics> characteristics() {
    return Collections.unmodifiableSet(EnumSet.of(Characteristics.IDENTITY_FINISH));
}

private static boolean isPrime(final int candidate) {
    return IntStream.rangeClosed(2, (int) Math.sqrt(candidate)).noneMatch(i -> candidate % i == 0);
}
}

及其用法:

@Test
public void collectingPrimeNumbersWithCustomCollector() {
    Map<Boolean, List<Integer>> primesNumbersMap = IntStream.rangeClosed(1, 1_000_000)
            .boxed()
            .collect(CustomCollectors.primeNumbers()); //or new PrimeNumbersCollector()

    Utils.printLine("Prime numbers between 1 - 1_000_000:");
    Utils.printLine(primesNumbersMap.get(Boolean.TRUE));
}

然后您可以 limit(1000)sum(0, BinaryOperator) 所有素数,直到达到计数限制。

或者,您可以使用以下方法将数字流过滤为 select 仅素数并对它们求和:

 private static boolean isPrime(final int candidate) {
        return IntStream.rangeClosed(2, (int) Math.sqrt(candidate)).noneMatch(i -> candidate % i == 0);
    }

用法如下所示:

Stream.iterate(1L, i -> i + 1) //iterate with sequential numbers
                .filter(MyFancyClass::isPrime)
                .limit(1000)
                .reduce(0L, Long::sum);

第二种方法比第一种更简洁高效。

希望这能回答您的问题。

不,使用 .get() 而不使用 .ifPresent 不一定是不好的做法。这归结为您正在实施的逻辑。如果空 Optional 是一个例外情况,那么依靠 .get() 抛出一个 NoSuchElementException 是完全合适的。

所以您应该问自己的问题是,如果 getFirstThousandPrimes() returns 一个空列表,您的代码究竟应该做什么。

在您的特定测试中,输入为空是完全有效的:零数字之和为零。所以你可以使用 .reduce(Integer::sum).orElse(0) 或完全摆脱像 .reduce(0, Integer::sum).

这样的选项

另请注意,您可以转换为原始流并直接使用 sum() 方法:

getFoos().stream().mapToInt(x -> x).sum();

如果输入为空,这样你当然也会得到 0。