GeezerVison is showing github.com — Generate getter methods for query models by Cyberax · Pull Request #387 · sqlc-dev/sqlc
Conversation
Cyberax
commented
Mar 9, 2020
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
This change allows the use of face interfaces to write somewhat generic
mapping methods.
Example:
// An example of an application-level structure
type AppAuthor struct {
ID int64
Name string
Bio string
}
// A mapper that uses a face interface
func MapToAuthor(a interface {
GetID() int64
GetName() string
GetBio() sql.NullString
}) AppAuthor {
return AppAuthor{
ID: a.GetID(),
Name: a.GetName(),
Bio: a.GetBio().String,
}
}
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This change allows the use of face interfaces to write somewhat generic
mapping methods.
Example:
// An example of an application-level structure
type AppAuthor struct {
ID int64
Name string
Bio string
}
// A mapper that uses a face interface
func MapToAuthor(a interface {
GetID() int64
GetName() string
GetBio() sql.NullString
}) AppAuthor {
return AppAuthor{
ID: a.GetID(),
Name: a.GetName(),
Bio: a.GetBio().String,
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Cyberax I'm so sorry this PR has languished with a review. I was going to review it and then things got a bit crazy.
I'm not sold on the utility of these Getter methods. They also conflict other fields on the structs themselves. You can imagine a database table with a foo and get_foo column.
If you're looking for these type of interfaces, I'd suggest using a small amount of codegen to inspect the struct and generate the methods from that.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
These "face interfaces" actually saved me quite a bit of code in mappers, because I have a fair amount of almost but not exactly the same queries.
I don't see any problems with foo and get_foo, you'll have methods GetFoo and GetGetFoo. This is admittedly ugly, but won't cause any issues.
I guess custom codegen is an option, but at least can you tags to generated classes so that they can be easily discovered?
|
ekovacs reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Would you mind revisiting this PR? It would really help us (we're maintaining an internal fork for now).
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
+1, this would be particularly useful for us as well for a similar reason!
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
I'd also find this convenient, but I think it's reasonable to do with a separate code gen tool. Seems to me that there's enough complexity here already, especially for a personal project - cutting scope to keep the project semi-maintainable seems like a good long-term decision.
I don't see any problems with foo and get_foo, you'll have methods GetFoo and GetGetFoo. This is admittedly ugly, but won't cause any issues.
The problem is that the get_foo column will (by default) correspond to a GetFoo struct field, which conflicts with the GetFoo accessor for the foo column. See "Method declarations" in the Go spec: "If the base type is a struct type, the non-blank method and field names must be distinct." This exact issue creates some complexity in the protobuf and Apache Thrift compilers, both of which generate accessors.
(Also, 👋 @rf!)
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
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.
|