German Locale Support #85
Conversation
There was a problem hiding this comment.
@rweisleder Thank you for your PR. It looks good, but I have added comments about some minor changes that it would be good to implement.
|
|
||
| @Override | ||
| public String get() { | ||
| return baseProducer.randomElement("L", "M", "N", "P", "R", "T", "V", "W", "X", "Y") + randomNumeric(8); |
There was a problem hiding this comment.
It would probably be better to extract it to a collection/ array.
| } | ||
|
|
||
| public boolean isValid(String vatIdentificationNumber) { | ||
| return vatIdentificationNumber.matches("^[0-9]{9}$"); |
There was a problem hiding this comment.
Please extract to constant.
| } | ||
|
|
||
| public boolean isValid(String passportNumber) { | ||
| return passportNumber.matches("^[CFGHJK][0-9]{8}$"); |
There was a problem hiding this comment.
Please extract to constant.
|
|
||
| @Override | ||
| public String get() { | ||
| return baseProducer.randomElement("C", "F", "G", "H", "J", "K") + randomNumeric(8); |
There was a problem hiding this comment.
Please extract to collection/ array.
|
|
||
| where: | ||
| vatIdentificationNumber | valid | ||
| "999999999" | true |
There was a problem hiding this comment.
Maybe use Groovy String in place of GString?
|
|
||
| def "should generate random street"() { | ||
| expect: | ||
| address.street == "Messelweg" |
There was a problem hiding this comment.
Please convert to Groovy String in place of GString. Please do so for all the GStrings where a Groovy String could be used instead.
|
|
||
| def "should return address in de locale format"() { | ||
| expect: | ||
| address.toString() == "Messelweg 176" + LINE_SEPARATOR + "15286 Schlitz" |
There was a problem hiding this comment.
Why not take advantage of GString concatenation, like so: "Messelweg 176${LINE_SEPARATOR}15286 Schlitz" ?
|
|
||
| where: | ||
| nationalIdentityCardNumber | valid | ||
| "T22000129" | true |
There was a problem hiding this comment.
Please convert to Groovy Strings in place of GString.
|
|
||
| where: | ||
| passportNumber | valid | ||
| "C22000129" | true |
There was a problem hiding this comment.
Please convert to Groovy Strings in place of GStrings.
Please remove unnecessary empty lines in test methods.
- replaced unnecessary GString with String - extracted constant patterns and collections - removed unnecessary white lines in tests
one GString has been hiding...
|
@OlgaMaciaszek thanks for your feedback. I updated my PR. |
This adds locale support for german (de)
The
DeNationalIdentityCardNumberProviderandDePassportNumberProvidergenerate only the simplest variants with a letter and eight digits. It would also possible to mix digits and characters (except A, E, I, O, U and B, D, Q, S) as long as they are eight-digit.