[refactor] extract picking a random enum value to method RandomService.nextEnum()#1530
[refactor] extract picking a random enum value to method RandomService.nextEnum()#1530kingthorin merged 4 commits intomainfrom
RandomService.nextEnum()#1530Conversation
PR Summary
|
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #1530 +/- ##
============================================
+ Coverage 92.42% 92.47% +0.04%
- Complexity 3273 3275 +2
============================================
Files 325 325
Lines 6440 6441 +1
Branches 625 625
============================================
+ Hits 5952 5956 +4
+ Misses 336 334 -2
+ Partials 152 151 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| return randomBytes; | ||
| } | ||
|
|
||
| public <T extends Enum<T>> T nextEnum(Class<T> klass) { |
There was a problem hiding this comment.
Since this is now a public method, would it be possible to add a unit test for this?
| } | ||
|
|
||
| public <T extends Enum<T>> T nextEnum(Class<T> klass) { | ||
| T[] enumConstants = klass.getEnumConstants(); |
There was a problem hiding this comment.
there is already existing method with similar code
datafaker/src/main/java/net/datafaker/providers/base/Options.java
Lines 146 to 149 in 646a3f9
probably it doesn't have obvious name since it is not the first time there is a PR with another method like that..
How about making one of them reusing another?
There was a problem hiding this comment.
@snuyanzin Good point, there was such a method. But it was hard to find because of its name doesn't contain "enum".
I hope now it got better.
the name "option" was not clearly indicating that it works with Enum.
| randomService.nextEnum(Ring.class), | ||
| randomService.nextEnum(Ring.class), | ||
| randomService.nextEnum(Ring.class) | ||
| )).isEqualTo(List.of(expected1, expected2, expected3, expected4, expected5)); |
There was a problem hiding this comment.
| )).isEqualTo(List.of(expected1, expected2, expected3, expected4, expected5)); | |
| )).containsExactly(expected1, expected2, expected3, expected4, expected5); |
| @Test | ||
| void nextEnum() { | ||
| RandomService randomService = new RandomService(); | ||
| Set<Ring> all = new HashSet<>(); |
There was a problem hiding this comment.
| Set<Ring> all = new HashSet<>(); | |
| Set<Ring> all = new EnumSet<>(); |
snuyanzin
left a comment
snuyanzin
left a comment
There was a problem hiding this comment.
LGTM
there is a couple of minor comments
5 participants
No description provided.