feat: Inject individual objects and validate the constructors#866
feat: Inject individual objects and validate the constructors#866lionel-nj merged 1 commit intoMobilityData:masterfrom
Conversation
aababilov
force-pushed
the
inject-date
branch
2 times, most recently
from
aac032d to
bcaf1f5
Compare
aababilov
changed the title
lionel-nj
left a comment
lionel-nj
left a comment
There was a problem hiding this comment.
Thanks @aababilov ! If I understand correctly the logic between tables and constructor parameters has been "inverted", .get method allows passing the correct parameter (CurrentDateTime or CountryCode. And there is an additional check to verify constructors are corretly defined?
| public static <T> T createValidatorWithContext( | ||
| Class<T> clazz, ValidationContext validationContext) throws ReflectiveOperationException { | ||
| return createValidator(clazz, parameter -> validationContext); | ||
| return createValidator(clazz, parameter -> validationContext.get(parameter)); |
| @@ -0,0 +1,12 @@ | |||
| package org.mobilitydata.gtfsvalidator.validator; | |||
There was a problem hiding this comment.
I suppose you might want to add a copyright header here :)
There was a problem hiding this comment.
Thanks @aababilov ! If I understand correctly the logic between tables and constructor parameters has been "inverted",
.getmethod allows passing the correct parameter (CurrentDateTimeorCountryCode. And there is an additional check to verify constructors are corretly defined?
Yes, that's right!
There was a problem hiding this comment.
I added the copyright :)
I work on a laptop and a desktop, so the laptop inserts copyrights automatically but the desktop does not yet :)
| parameterClass.isAssignableFrom(ValidationContext.class) ? validationContext : table); | ||
| parameterClass.isAssignableFrom(table.getClass()) | ||
| ? table | ||
| : validationContext.get(parameterClass)); |
1. Inject individual objects like CurrentDateTime instead of the whole ValidationContext. This makes dependencies more explicit since a validator requires just a certain part of the potentially large ValidationContext. 2. Validate that all validator constructors accept parameters of supported types and throw an exception for any invalid validator. This prevents hidden bugs when a validator cannot be instantiated but nobody notices that because a file for the corresponding validator is not present.
|
Thanks for review! I addressed your comment and resolved merge conflicts. Can we merge that now? |
2 participants
of the whole ValidationContext. This makes dependencies
more explicit since a validator requires just a
certain part of the potentially large ValidationContext.
of supported types and throw an exception for any invalid
validator. This prevents hidden bugs when a validator cannot
be instantiated but nobody notices that because a file for
the corresponding validator is not present.