#1214 [refactoring] load country-specific ID number providers by country code #1229
Conversation
…try code Thus, we can add ID numbers for new countries just by implementing "IdNumbers" without need to add public methods like "validZhCNSsn()", "validEstonianPersonalCode()", "validBulgarianPersonalCode()" to class "IdNumber". Now all country-specific ID number generators are loaded from file "src/main/resources/META-INF/services/net.datafaker.idnumbers.IdNumbers" using a standard Java mechanism "service loader". + rename existing "*IdNumber" classes to human-readable, e.g. "ZhCnIdNumber" to "ChineseIdNumber".
| /** | ||
| * @return ISO-2 code of the country ("US" for America, "EE" for Estonia etc.) | ||
| */ | ||
| String country(); |
There was a problem hiding this comment.
would countryCode be perhaps a better named method? A method like country() might return "America" or so.
There was a problem hiding this comment.
Also, shouldn't this class perhaps be called "IdNumber", instead of "IdNumbers" (so, without the s?)
update: ah, I see that we already have an IdNumber class. Okay, maybe ignore this comment then, or maybe IdNumberGenerator ?
There was a problem hiding this comment.
@bodiam I like both variants: countryCode and IdNmberGenerator. Renamed.
|
I left a tiny comment, but really nice PR, thanks for that! |
kingthorin
left a comment
kingthorin
left a comment
There was a problem hiding this comment.
I didn't get through the whole PR
| } | ||
|
|
||
| public String getValid(BaseProviders faker) { | ||
| public String generateValid(BaseProviders faker) { |
There was a problem hiding this comment.
This will break for anyone using it directly. We should deprecate for removal and it can call the new method.
There was a problem hiding this comment.
Also shouldn't generate valid have an override annotation?
There was a problem hiding this comment.
I guess my original comment here goes for any of these country classes
There was a problem hiding this comment.
This will break for anyone using it directly
Generally it's a valid point, but not in this case.
These 5 countries were added recently by me (Estonia, Albania, Bulgaria, Macedonia, Moldova), and not yet released. So they are not yet used by anybody.
Also shouldn't generate valid have an override annotation?
Yes, good point. I've added missing @Override annotations and enabled corresponding inspection in IDEA.
| } | ||
|
|
||
| /** | ||
| * Returns a random String element from an varargs. |
There was a problem hiding this comment.
I guess it could be not only String
There was a problem hiding this comment.
I guess it could be not only String
@snuyanzin Good point, fixed javadoc.
UPD I removed this method at all, used nextElement instead.
| public <T> T option(List<T> options) { | ||
| return options.get(faker.random().nextInt(options.size())); | ||
| } |
There was a problem hiding this comment.
At the very end of this class there is already existing method
/**
* Returns a random element from a list.
*
* @param list The list to take a random element from.
* @param <E> The type of the elements in the list.
* @return A randomly selected element from the list.
*/
public <E> E nextElement(List<E> list) {
return list.get(faker.random().nextInt(list.size()));
}will not it help?
We need a separate for String because it could be used in expressions, however it's impossible to use generics in expressions... at least right now
There was a problem hiding this comment.
@snuyanzin Yes, indeed, I had to use this method. Thanks, fixed.
... instead of introducing a new method.
asolntsev
force-pushed
the
refactoring/1214-load-country-specific-providers-by-country-code
branch
from
bf3717f to
c712b4f
Compare
|
LGTM2! |
asolntsev
deleted the
refactoring/1214-load-country-specific-providers-by-country-code
branch
4 participants
Sorry for too big PR.
I just wanted to remove unneeded public method before we release a new version and people start using them.
Thus, we can add ID numbers for new countries just by implementing "IdNumbers" without need to add public methods like "validZhCNSsn()", "validEstonianPersonalCode()", "validBulgarianPersonalCode()" to class "IdNumber".
Now all country-specific ID number generators are loaded from file "src/main/resources/META-INF/services/net.datafaker.idnumbers.IdNumbers" using a standard Java mechanism "service loader".