dont use factories

[arishlabroo]

We got feedback that use of a factory function in go is not "idiomatic". I would like to use this issue to understand this more and gather feedback/alternatives.

To clarify, we don't "need" factories. Here is a pull request without a factory for the server, while still keeping the server testable and configurable. https://github.com/zillow/howwegoatzillow/pull/1/files

basically we added a Configure() method on the Server. And now you can simply do

srvr:= Server{}
srvr.Configure(...options) // in the base package for common functionality, default logger etc
srvr.Configure(...options) // in your service for service specific functionality like customHealthCheck

instead of

f := NewFactory(...options) // in the base package for common functionality, default logger etc
srvr:=f.Create(...options) // in your service for service specific functionality like customHealthCheck

couple of things that standout in the factory less approach.

1. Every service will need a new type to represent the application specific server.

1. https://github.com/zillow/howwegoatzillow/blob/no-factory/app/server.go#L12
2. This is because you cannot have multiple providers for the same type. One in the common packages(base zillow server), and one in each repo (server with service specific functionality). Wire has no way to follow which initializer to use for what.
3. This is a wire problem, don't use wire
   1. 🤷‍♂️
4. Do you need an initializer for your service specific server? instead of func NewMyServer(server *server.Server, service *MyService) *MyServer why cant you do func DoMyServerThings(server *server.Server, service *MyService) {
   1. Who is going to call this? Is the main now responsible for getting the Server and MyService and then call this function before it starts serving?
   2. How do you do integration testing now? If you want to include the transport layer in integration tests, In each test, after you get a testable server and testable MyService, you also have to remember to call this function before you can proceed with the tests?

2. Ideally the server after creation is immutable.

1. With a factory, after you have created a Server, there is nothing you can do on the it other than Serve().
2. With the Configure() method, imagine this scenario

srvr.Configure(...options)
go func() { _ = srvr.Serve() }()
srvr.Configure(...options)
<-os.Signal

What does the second Configure do?

3. error, based on what, a mtx and a boolean flag inside to server to prevent further configuration?
4. ignore? yeah, thats what http.Server does . Does it?

srvr := &http.Server{Addr: ":8080"}
go func() { _ = srvr.ListenAndServe() }()
srvr.Addr = ":8081"
<-os.Signal

5. Is this guaranteed to listen on 8080 🤔 ? What if the scheduler hasn't gotten to the ListenAndServer go routine yet 🤔
6. This is a BS scenario. absolutely not worth accounting for.
   1. 100% agree.
   2. But if the server was not mutable in the first place, we would not even have to talk about how BS this usage is.

---

These two issues are in no means an attempt to demonstrate that we need Factories. I 100% agree that we don't "need" them.

I am more curious about whats wrong with using them 🤔 . (factoryOfFactory or other java jokes aside), and more importantly, what alternative approach would you recommend? (ps: none of our services have factories, we have never seen a need for them outside of couple of core packages)

[stewartboyd119]

Factory upside:

1. server.Server only has two methods for its public interface. Both having to do with serving.
2. Reasoning about the setup of your server instance involves looking at where the factory called create (and the call to the factory provider to see what options it was passed that it'll pass on). This is the benefit of the private fields of zserver.Server and that there's no public constructor

Factory downside:

1. Not as intuitive. Likely dev will try and instantiate server.Server directly (realize everything is private), then look for a provider (NewServer), and only after not finding this maybe glance upon factory and realize that's the best way to create the server.Server
2. The factory adds a layer of indirection complicating the usage model. For example, to impact how the server.Server is created you pass args into NewFactory, factory.Create(). All of these are options that have to get translated into field changes on server.Server (weakening of cohesion).

I understand why, for general consumption, there'd be some pushback. For your organization however, there are likely plenty of examples of usage, as well as coaching from the author's that reduce the importance of the intuition piece. In that context, you're less worried about wide spread adoption and more concerned about isolating bugs and the general debugging experience. Considering that, it makes sense that you'd make the tradeoff in favor of factories.