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

All methods on models should use pointer receivers #1272

Open
Mjaethers opened this issue Dec 19, 2023 · 2 comments · May be fixed by #1325
Open

All methods on models should use pointer receivers #1272

Mjaethers opened this issue Dec 19, 2023 · 2 comments · May be fixed by #1325

Comments

@Mjaethers
Copy link
Contributor

Mjaethers commented Dec 19, 2023

Is your feature request related to a problem? Please describe.
Goland has been harassing me about the fact that some structs in the package model mix value and pointer receivers. This is not recommended by the Go documentation.

Describe the solution you'd like
It would make sense to convert the methods with pointer receivers to value receiving methods as the mutability provided by using pointers is achieved through the dao design pattern.
It would make sense to convert the methods with value receivers to pointer receiving methods as this would be more efficient for large structs.

Describe alternatives you've considered
It would also be possible to convert the methods with value receivers to methods with pointer receivers, the docs suggest this would be more efficient but since most methods use value receivers and the commitment to the dao design pattern was made, I think using value receivers makes more sense.
It could either be left as is or the dao pattern could be extended for the model package.

Additional context
The Go docs talk about this in their FAQ here

@joschahenningsen
Copy link
Member

I agree. However, some models have methods that mutate the model, so we should be careful not to break anything.

@Mjaethers
Copy link
Contributor Author

Mjaethers commented Feb 1, 2024

I had a second look at this and realised that dao is only used by the api package and not the model package. So I see no reason for methods on models to use value receivers at all. Aside from consistency, I think that copying rather large structs for often fairly simple methods is a bit inefficient.
I doubt this would make a significant difference in performance, but if there's no reason to use value receivers then it might as well be changed.

@Mjaethers Mjaethers changed the title All methods on models should use value receivers All methods on models should use pointer receivers Feb 1, 2024
@Mjaethers Mjaethers linked a pull request Feb 1, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants