DatabaseAdapter value object #1671
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It's meant to replace string literals across codebase, especially across several case statements.
This design is closer to actual use–case. VO will be instantiated based on string literal containing adapter name.
Use of ArgumentError might be tempting, but it should be used in a bit different context. Implementing specific error is a bit nicer approach.
When trying to instantiate unsupported adapter, exception will be raised. No need to call class to make yet another string comparison.
Object is always a better abstraction than string.
It can have less responsibilities now.
Not to confuse each other.
DatabaseAdapter can now receive data_type as argument. Each specific adapter contains list of allowed data types and won't allow use of unsupported one.
However, on the class level it will be hard to determine current adapter. Let's display all the available types (PostgreSQL has biggest set of such) and fail further if incompatible type is used.
Same features, less verbose implementation. Introducing new adapter type should be easier now. Co-authored-by: Piotr Jurewicz <[email protected]>
Maybe it would make sense, but verification of data type would require a lot of gymnastics and we see no value in it. Co-authored-by: Piotr Jurewicz <[email protected]>
We don't need to reflect specific adapter naming. Default driver for MySQL in rails 7.1 will be Trilogy. It's a detail not in our scope of interest. We only need to be sure which data types are allowed for given database. Co-authored-by: Piotr Jurewicz <[email protected]>
It will be a default in Rails 7.1. In our scope it's yet another MySQL adapter and check only matters when allowing specific data types supported by database itself. Co-authored-by: Piotr Jurewicz <[email protected]>
Template directory depends on used adapter type, so it's a viable place to keep this information close to the adapter itself. Yet another case statement removed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Btw. it enables use of
tridentmysql adapter