gistfile1.md

in re: weaveworks/scope#515 (comment)

Whats the downsides of this approach? I'm not a huge fan of the factory style, if only because it pollutes potentially clean APIs (it would work better if golang had optional parameters...)

The downside is that it pollutes the global namespace—all else equal, our goal should be to have no global state—and makes testing harder, i.e. this

oldStubThing := stubThing
defer func() { stubThing = oldStubThing }()
stubThing = func() Thing { return mockThing{123} }

c := newComponentActuallyUnderTest(...)

is worse than this

f := func() Thing { return mockThing{123} }
c := newComponentActuallyUnderTest(f, ...)

not only because it's more verbose, but because the dependency is spooky action at a distance (poke global state here, poke other thing there, assume they know about each other) rather than declarative (constructor depends on its parameters).

It's especially bad when the thing being passed in is sufficiently specialized to "make sense" in the context of the owning component. So, time.Now or time.After probably doesn't make sense to treat this way, as those are generally totally orthogonal to the WhateverManager that uses them. But in this instance, a conntracker factory is pretty closely coupled.

Maybe a rule that approximates this behavior could be: if it's in the stdlib, it's fine to mock, but if it's our code, it should be passed as a dependency?

And I will pretend I didn't hear you advocate in favor of optional parameters :)