Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generate getter methods for query models #387

Closed
wants to merge 2 commits into from
Closed

Generate getter methods for query models #387

wants to merge 2 commits into from

Conversation

Cyberax
Copy link
Contributor

@Cyberax Cyberax commented Mar 9, 2020

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,
	}
}

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,
	}
}
@kyleconroy kyleconroy self-requested a review March 9, 2020 20:40
Copy link
Collaborator

@kyleconroy kyleconroy left a 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.

@Cyberax
Copy link
Contributor Author

Cyberax commented Apr 2, 2020

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?

@Cyberax
Copy link
Contributor Author

Cyberax commented May 3, 2020

Would you mind revisiting this PR? It would really help us (we're maintaining an internal fork for now).

@rf
Copy link

rf commented May 18, 2020

+1, this would be particularly useful for us as well for a similar reason!

@akshayjshah
Copy link
Contributor

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!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants