search

eduvpn / eduvpn-common

Code to be shared between eduVPN clients
MIT License
5 stars 4 forks source link

Client implementation accross files #3

Closed peske closed 1 year ago

peske commented 1 year ago

As far as I can see, Client structure is implemented in https://github.com/eduvpn/eduvpn-common/blob/main/client/client.go . Many of its receiver functions (methods) are also implemented there. But for some reason there are few receiver functions implemented in https://github.com/eduvpn/eduvpn-common/blob/main/client/fsm.go . For example:

https://github.com/eduvpn/eduvpn-common/blob/6229767a7d5b0d3cf3125f2e9da0ca0e66ada814/client/fsm.go#L255

It is a bad practice to define receiver functions of the same type in multiple files. Is there any reason for such implementation?

jwijenbergh commented 1 year ago

It's because of the fact that otherwise the file becomes a bit too big to deal with. I had it in one file before (e.g. https://github.com/eduvpn/eduvpn-common/blob/242903aa810797102b14e27dda988fff7ab833cc/client.go). Maybe right now it won't be so bad (~1k lines), but it becomes harder to also group the relevant functionality together.

Maybe the best way forward is that I try to split some of these functions into a new struct (so they don't take the client as receiver). Do you have any idea? I hate the fact that e.g. SetSearchServer even takes a client in the first place.

jwijenbergh commented 1 year ago

no longer the case in v2