TUM-Dev / gocast

TUMs lecture streaming service.
https://live.rbg.tum.de/
MIT License
177 stars 41 forks source link

All methods on models should use pointer receivers #1272

Open Mjaethers opened 6 months ago

Mjaethers commented 6 months ago

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 commented 6 months ago

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

Mjaethers commented 5 months ago

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.