Add null-check in RandomStringGenerator#selectFrom() to avoid NullPointerException #434
Conversation
Codecov Report
@@ Coverage Diff @@
## master #434 +/- ##
=========================================
Coverage 97.14% 97.14%
Complexity 2333 2333
=========================================
Files 84 84
Lines 5780 5781 +1
Branches 936 937 +1
=========================================
+ Hits 5615 5616 +1
Misses 87 87
Partials 78 78
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| characterList = new ArrayList<>(); | ||
| for (final char c : chars) { | ||
| characterList.add(c); | ||
| if(chars != null) { |
There was a problem hiding this comment.
Checkstyle issues in the PR (you can run mvn locally and that should do the same as GH Actions).
Error: src/main/java/org/apache/commons/text/RandomStringGenerator.java:[244,13] (whitespace) WhitespaceAfter: 'if' is not followed by whitespace.There was a problem hiding this comment.
I got it. Yes. I see the error locally. Thank you
| final String str = "abc"; | ||
| final RandomStringGenerator generator = new RandomStringGenerator.Builder().selectFrom(null).build(); | ||
| final String randomText = generator.generate(5); | ||
| assertThat(randomText.isEmpty()).isFalse(); |
There was a problem hiding this comment.
You need to assert the results are correct, not that any old output was generated.
| for (final char c : chars) { | ||
| characterList.add(c); | ||
| if (chars != null) { | ||
| characterList = new ArrayList<>(); |
There was a problem hiding this comment.
-1: This PR breaks the documented contract in the Javadoc (the contract is still broken for null). This is unexpected because the consequence of null input should be the same as empty (size 0) input but in fact, is not.
For example:
builder.
selectFrom('a')
selectFrom();
gives us a characterList of [] but:
builder.
selectFrom('a')
selectFrom(null);
gives us a characterList of ['a'] instead of [].
IOW, null input and empty should be the same. If not, why not? Or am I missing something? Why doesn't selectFrom() always reset characterList?
…alled, regardless of the method input --I changed the test to assert the length of the random text
…to TEXT-make-selectFrom-null-safe-RandomStringGenerator
garydgregory
changed the title
orionlibs
deleted the
TEXT-make-selectFrom-null-safe-RandomStringGenerator
branch
No description provided.