Hacker News new | past | comments | ask | show | jobs | submit login

I really like Mat Ryer's work, and I've applied most of the ideas in the 2018 version of this article to all of my Go projects since then.

The one weak spot for me is this aspect:

>NewServer is a big constructor that takes in all dependencies as arguments... In test cases that don’t need all of the dependencies, I pass in nil as a signal that it won’t be used.

This has always felt wrong to me, but I've never been able to figure out a better solution.

It means that a huge chunk of your code has a huge amount of unnecessary shared state.

I often end up writing HTTP handlers that only need access to a tiny amount of the shared state. Like the HTTP handler needs to check if the requesting user has access to a resource, and then it needs to call one function on the datastore.

I'd love to write tests where I only mock out those two methods, but I can't write simple tests because the handler is part of this giant glob where it has access to all of the datastore and every object the parent server has access to because it's all one giant object.

Nothing against Mat Ryer, as his pattern is the best I've found, but I still feel like there's some better solution out there.




I've become increasingly sensitive to these high afferent coupling points in the repos I work on, especially the deeper I embed into the world of bazel and how dependency management and physical design influence the code I author.

Where possible plugins are a great strategy to lay down these code seam points that don't force all possibilities upon some body of code, because fundamentally with plugin architectures you pick and choose what you want. Plugins are opt out by default, you must explicitly opt into a plugin for it to manifest. I've been calling software that has this quality going as being an "a la carte" style.

But in general you do what you need to do to avoid "doing everything so you can do anything".


I tend to write most of my logic in packages... so a "users" package or a "comments" package (if we were building HN). These have NO http interface! They do however each have their own "main" and some sort of CLI interface: "//go:build ignore" in the comment of that file is your friend.


I just use a closure. So instead of defining a handler as:

  func HandleX(w http.ResponseWriter, req *http.Request) {
    // code
  }
I use:

  func HandleX(store *DataStore, dep1 Foo, dep2 Bar, commonDep Common) http.HandlerFunc {
    //
    // maybe some initialization
    //
    return func(w http.ResponseWriter, req *http.Request) {
      // code
    }
  }
and initialize them all once in whatever entrypoint there is.


It means the object created by NewServer is dealing with too much. Probably has too many data types coupled to it and too much behavior.

Simple example is adding a logger. If you add it as a dependency to the constructor, the object starts doing a bit more than initial simple implementation. It's fine to do it, but shame to not figure out how to log without editing the implementation of a simple thing.

Higher order functions (a logger decorator) get there to allow composition, but even they have their drawbacks.

It's still some form of structure that you can deal with, not a mistake.


As you say, having a logger attached is one of those pragmatic and acceptable exceptions to the rule. In a perfect world we'd have the time to go to the trouble of implementing loggable types and data flows and associated higher order functions, in practice taking the compromise means getting the real business valuable work completed while still having the necessary (but usually "low priority") non-functional requirements like logging and metrics implemented.


I agree that too many arguments to the constructor may have the smell of too much coupling.

But if I really feel I can't avoid the need to pass a good amount of external context, I create a dedicated "options" struct and pass that into the constructor as a pointer. The purpose of the pointer (rather than pass by value) is if I want default arguments, I can pass nil.

    type ServerOptions struct {
        logger    *magic.Logger
        secretKey string
    }

    func NewServer(options *ServerOptions) (*Server, error) {
        ...
    }


I felt this way for a long time. And maybe I'm projecting my past struggles onto what you're describing. I shared my current approach in a different comment already [0]. The gist is that I use an optional config struct, whose values get validated and copied over to my server struct inside NewServer. This makes testing much easier because I can mock fewer deps.

FWIW, I really tried to make the functional option pattern work for me, as many others have suggested, but eventually abandoned it. I felt it was a little too clever and therefore difficult to read, while requiring more boilerplate than the config struct + validate and copy pattern.

[0] https://news.ycombinator.com/item?id=39320170


Maybe this is heresy in the Go community, but this is a problem that automatic dependency injection solves.

https://dagger.dev/hilt/testing-philosophy.html

The biggest problem with the "pass nil for unused dependencies" approach is that when you modify some code to actually use one of those dependencies when it didn't before, you have to go back through every test and populate it.


Automatic dependency injection doesn’t solve that, it masks it and makes the onset of pain from poor design appear much later than it would without - by which time it is harder to fix.


My experience has been the opposite - we've been maintaining large (tens of millions loc, thousands of developers) mobile app codebases with this approach for about 8 years now, and results have been much better than what was done before.

A number of bad decisions were made and later discovered over time, and it was much easier to address them with automatic dependency injection available than without. Using as "big" of a test as you can without regressing speed or flakiness gives you really solid, non-fragile tests, which makes large changes tractable to do safely.

I don't know whether certain poor designs would have been spotted sooner, but this certainly made the code easier to change and resulted in more useful and lower maintenance tests.



> It means that a huge chunk of your code has a huge amount of unnecessary shared state.

Can you explain that a little more?

Which chunk of code has what shared state, and why is it unnecessary?


Basically, not all the handlers will use every dependency the server (which is the entire program in this pattern) has. Not every handler will use a database, for example.

While I may prefer a struct for this instead of separate arguments, I do agree it's useful to capture "the world" as the set of all dependencies, even if some handlers don't use them (yet).


You can use Dependency Injection to solve this issue but in my view the added complexity is not really worth it.


Is this a Go thing? In C# land this is trivial.


There are Go DI frameworks overall DI is not that popular in Go community.




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: