feat/chainable translators #542
Conversation
|
|
||
| @Override | ||
| public String apply(String sql) { | ||
| if (this.translators.size() == 1) { |
There was a problem hiding this comment.
Regarding readability / maintenance:
Is this special handling for size == 1 really needed?
Yes, the iterator is not invoked here and it might have performance benefits but shouldn't take the JVM care of optimising this?
There was a problem hiding this comment.
Burned by this too many times wrt streams.
| } | ||
|
|
||
| private int getOrder(SqlTranslator sqlTranslator) { | ||
| if (sqlTranslator != null) { |
There was a problem hiding this comment.
At this point the SqlTranslator should never be null.
Maybe a check in the public compare method for this with potential exception throwing. I mean even Java built-in types have no nice mitigation for null values on compare.
What might be checked for null is the value of SqlTranslator#getOrder because it can be overwritten to return null.
There was a problem hiding this comment.
It can't return null, only 0, it's an int.
There was a problem hiding this comment.
🤦 myself..nothing to see here 😄
| The former is provided for two reasons: It allows us to distribute the driver with and without the bundled, default translator and can be an option for you to run your custom translator. | ||
|
|
||
| Translators can be chained, and you can have as many translators on the class-path as you want. |
There was a problem hiding this comment.
| Translators can be chained, and you can have as many translators on the class-path as you want. | |
| Translators can be chained, and you can have as many translators on the class path as you want. |
Also note that there is as well classpath in the current documentation. This should get unified.
There was a problem hiding this comment.
Most official docs use classpath (Edit: Geez autocorrect)
There was a problem hiding this comment.
Then my suggestion is of course
| Translators can be chained, and you can have as many translators on the class-path as you want. | |
| Translators can be chained, and you can have as many translators on the classpath as you want. |
😺
michael-simons
force-pushed
the
feat/chainable-translators
branch
2 times, most recently
from
1877401 to
445797e
Compare
SQL translators now can be ordered and you can have as many SQL translator factories as you want on the class- or module-path. The will be called in order of highest precedence, and individual translators but the last are free to fail or to modify the intermediate statement. No additional translators are provided in this commit. Co-authored-by: Gerrit Meier <[email protected]>
michael-simons
force-pushed
the
feat/chainable-translators
branch
from
d289010 to
641daa4
Compare
@sincetags.