Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use a immutable 'characterList' to receive characters passing by with… #125

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ijliym
Copy link

@ijliym ijliym commented Nov 11, 2019

…inRange and selectFrom to let method chaining is meaningful

char[][] CHAR_PAIRS = {{'0', '9'}, {'A', 'Z'}, {'a', 'z'}};
GENERATOR = new RandomStringGenerator.Builder()
        .withinRange(CHAR_PAIRS)
        .selectFrom(',', ';', ':', '?', '*', '#', '!')
        .build();

GENERATOR.generate(12) ==> "9gPhj2WaT;8O" (a random password)

I think, multiple calls selectFrom method to replace the previously stored Character is no use.

…inRange and selectFrom to let method chaining is meaningful

```java
GENERATOR = new RandomStringGenerator.Builder()
        .withinRange(CHAR_PAIRS)
        .selectFrom(',', ';', ':', '?', '*', '#', '!')
        .build();
```
@coveralls
Copy link

coveralls commented Nov 11, 2019

Coverage Status

Coverage decreased (-0.0004%) to 97.981% when pulling dc42a99 on ijliym:master into 8e31edf on apache:master.

@garydgregory
Copy link
Member

@ijliym
So the list grows forever? That seems like a change in behavior and I'm not sure it's right.

Copy link
Member

@garydgregory garydgregory left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my previous question. In general, don't mix style changes with behavior changes in your PRs, it makes them harder to review, so remove them, please.

@@ -247,6 +247,7 @@ public String generate(final int minLengthInclusive, final int maxLengthInclusiv
* Some commonly used predicates are provided by the {@link CharacterPredicates} enum.</p>
*
* <p>This class is not thread safe.</p>
*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for extra whitespace here.

@@ -328,7 +329,7 @@ public Builder withinRange(final int minimumCodePoint, final int maximumCodePoin
* Specifies the array of minimum and maximum char allowed in the
* generated string.
* </p>
*
* <p>
* For example:
* <pre>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you open a p tag above, then close it before this pre tag.

* @return The configured {@code RandomStringGenerator}
*/
@Override
public RandomStringGenerator build() {
return new RandomStringGenerator(minimumCodePoint, maximumCodePoint, inclusivePredicates,
random, characterList);
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't add whitespace here.

}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't add whitespace here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants