min() max() now throw NPE on null values #1692
Conversation
Current coverage is 97.06% (diff: 100%)@@ master #1692 diff @@
==========================================
Files 89 89
Lines 11247 11247
Methods 0 0
Messages 0 0
Branches 1888 1888
==========================================
Hits 10917 10917
- Misses 185 187 +2
+ Partials 145 143 -2
|
| } else { | ||
| return TreeSet.ofAll(toStringComparator(), Iterator.of(elements)); | ||
| } | ||
| return TreeSet.<T> of(naturalComparator(), elements); |
There was a problem hiding this comment.
@pivovarit do you have a hint how I can tweak Sputnik's default rules to accept this whitespace?
There was a problem hiding this comment.
There is a way of overriding the default config by adding some files locally. There it should be possible to tweak it.
@pjagielski can you help?
There was a problem hiding this comment.
Yes, under project' settings, there is a "I have my own config" checkbox which allows you to select a file from repo to use as checkstyle/findbugs/pmd config:
There was a problem hiding this comment.
@pjagielski Thank you, that helps!
|
@pivovarit is there a way to perform the Sputnik checks locally in order to get faster turn-around cycles? |
|
|
||
| public abstract class AbstractSortedSetTest extends AbstractSetTest { | ||
|
|
||
| @Override | ||
| abstract protected <T> SortedSet<T> of(T element); | ||
|
|
||
| abstract protected <T> SortedSet<T> of(Comparator<? super T> comparator, T element); |
There was a problem hiding this comment.
[Checkstyle] ERROR: 'protected' modifier out of order with the JLS suggestions.
| @Override | ||
| protected <T> BitSet<T> of(Comparator<? super T> comparator, T element) { | ||
| // comparator is not used | ||
| return this.<T> bsBuilder().of(element); |
There was a problem hiding this comment.
[Checkstyle] ERROR: '>' is followed by whitespace.
There was a problem hiding this comment.
Should be Synchronized with the Idea styles :)
|
|
||
| private final java.util.Map<Integer, T> fromIntMap = new java.util.HashMap<>(); | ||
| private final java.util.Map<T, Integer> toIntMap = new java.util.HashMap<>(); | ||
| private int nextValue = 0; |
There was a problem hiding this comment.
[Checkstyle] ERROR: Variable 'nextValue' explicitly initialized to '0' (default value for its type).
| } else { | ||
| return TreeSet.ofAll(toStringComparator(), Iterator.of(elements)); | ||
| } | ||
| return TreeSet.<T> of(naturalComparator(), elements); |
There was a problem hiding this comment.
[Checkstyle] ERROR: '>' is followed by whitespace.
| private Sieve() { | ||
| } | ||
|
|
||
| private final static List<Function2<Integer, Integer, Option<Integer>>> RULES = List.of( |
There was a problem hiding this comment.
[Checkstyle] ERROR: 'static' modifier out of order with the JLS suggestions.
| (x, y) -> Option.of((4 * x * x) + (y * y)).filter(n -> n % 12 == 1 || n % 12 == 5), | ||
| (x, y) -> Option.of((3 * x * x) + (y * y)).filter(n -> n % 12 == 7), | ||
| (x, y) -> Option.of((3 * x * x) - (y * y)).filter(n -> x > y && n % 12 == 11) | ||
| ); | ||
|
|
||
| final static List<Function3<Set<Integer>, Integer, Integer, Set<Integer>>> steps = List.of( | ||
| private final static List<Function3<Set<Integer>, Integer, Integer, Set<Integer>>> STEPS = List.of( |
There was a problem hiding this comment.
[Checkstyle] ERROR: 'static' modifier out of order with the JLS suggestions.
|
The remaining Sputnik remarks are ok for now - need to tweak the rules a bit. |
| @@ -968,6 +968,16 @@ public void shouldComputeMaxOfBigDecimal() { | |||
| assertThat(of(BigDecimal.ZERO, BigDecimal.ONE).max()).isEqualTo(Option.some(BigDecimal.ONE)); | |||
| } | |||
|
|
|||
| @Test(expected = NullPointerException.class) | |||
| public void shouldThrowNPEWhenMaxOfNullAndInt() { | |||
| System.out.println(of(null, 1).max()); | |||
There was a problem hiding this comment.
I think you can get rid of the System.out.println
|
|
||
| @Test(expected = NullPointerException.class) | ||
| public void shouldThrowNPEWhenMaxOfIntAndNull() { | ||
| System.out.println(of(1, null).max()); |
|
|
||
| @Test(expected = NullPointerException.class) | ||
| public void shouldThrowNPEWhenMinOfIntAndNull() { | ||
| System.out.println(of(1, null).min()); |
| @Override | ||
| protected <T> BitSet<T> of(Comparator<? super T> comparator, T element) { | ||
| // comparator is not used | ||
| return this.<T> bsBuilder().of(element); |
There was a problem hiding this comment.
Should be Synchronized with the Idea styles :)
| @Test | ||
| public void shouldNarrowQueue() { | ||
| final PriorityQueue<Double> doubles = of(1.0d); | ||
| public final void shouldNarrowPriorityQueue() { |
There was a problem hiding this comment.
Yes, it tests a static method of PriorityQueue. The whole test classes which contains tests for final classes should be also final (HashMapTest, TreeMapTest, ...)
| return factors(l) | ||
| .filter((d) -> d < l); | ||
| static Stream<Long> divisors(long l) { | ||
| return factors(l).filter((d) -> d < l); |
|
|
||
| Random RND = new Random(); | ||
| private static final Random RND = new Random(); |
There was a problem hiding this comment.
could also be: RND = ThreadLocalRandom.current();
|
@danieldietrich running Sputnik locally is not possible but you can definitely run those tools that Sputnik uses (those can be included as a part of the build). Before doing this, it's good to create a local config for those tools (which will help with the whitespace problem mentioned earlier) |
Fixes #1482