Yep - return concrete types, but also make sure you have an assertion along the lines of:

    var _ IfaceType = &ConcreteType{}

Somewhere, else you risk not knowing that a change to ConcreteType broke is implementation of IfaceType in a way that you might not know about until a rare runtime code path is executed.

> Somewhere, else you risk not knowing that a change to ConcreteType broke is implementation of IfaceType in a way that you might not know about until a rare runtime code path is executed.

Why would runtime be involved? Surely if ConcreteType doesn't satisfy the interface anymore then the compiler will catch that at any site where a ConcreteType is being used as / cast to an IfaceType?

The compiler will catch it at the point of use, but it's clearer if the error is at the point of declaration. Consider:

   type FooReader struct{}
   var _ io.Reader = FooReader{}

   func (r FooReader) Raed(p []byte) (int, error) { return 0, nil }

Before you even use FooReader as an io.Reader, you can find that you typo'd "Read". This is useful in practice because you probably wrote FooReader to have a bunch of other methods that aren't implementing io.Reader, and it's possible that your tests for this object don't actually ever use it as an io.Reader. So you won't notice this mistake until someone tries it elsewhere in the codebase.

It also acts as documentation that you intend for this thing to be an io.Reader. But I also recommend that you document Read as "// Read implements io.Reader." to be extra explicit.

Hah, how’s that for an endorsement of structurally typed interfaces.

That rule is not without its issues when pointers get involved, due to the typed nil problem.

Yeah, this is a setup for hard to understand bugs unless you have a corresponding rule (imposed by a linter) that a concrete type cannot be cast into an interface and then checked against nil (mostly this means when returning a concrete type from a function it should be assigned to a freshly declared variable rather than re-using a variable that could already have an interface type.

In practice this never happens unless you're violating other critical best practices like explicitly ignoring errors from constructors and trying to use the (nil) returned value anyway.

I think it doesn’t happen for errors because the error interface is almost always what is returned. It doesn’t seem that hard across if this style guide were followed: https://news.ycombinator.com/item?id=33566422

Typed nil isn't a problem, it indicates a crucial lack of understanding about interfaces, which should be resolved with education, not restricting good API design.

Interfaces are the most misused feature of Go. I think people like to prototype their API by typing in all the functions they intend to implement, so the interface acts as a template for their program that they can fill out. This, unfortunately, is not what they're for.

I have a longer rant about this here: https://jrock.us/posts/go-interfaces/

Thank you for sharing this.

I was struggling with this just today: Realizing I keep putting off writing tests for some modules because mocking a certain mega interface is a pain.

Your post gives me some great hints for things to try. Looking forward to experimenting with it.

It took me a long time to figure out how to make small interfaces. I had an "aha!" moment not too long before writing that post. It seems bad, then it suddenly makes sense, and then the other way seems bad.

Adding private members to my structs for testing was something the Go team forced me kicking and screaming to do when I worked at Google. I remember writing this adaptor for some internal network filesystem, and had an interface and two implementations: one that used the network for real, one that did everything in-memory for tests. I sent it off for code review and they were like "nope! don't do that in go!" and suggested an if statement in every method to handle the in-memory implementation. I was unhappy about it for months because it went against everything I knew about programming at the time, but like 10 years later, I appreciate that they were right. (I will say that everyone I explain this to has approximately the reaction that I had at the time. It is an easier pill to swallow when the person telling you not to do it wrote Go, but they don't have that benefit ;)

Really interesting approach. Thanks for sharing. Do you remember by any case where to find any real example? I'm digging into Golang's repositories trying to find one...

Wow, I find this quite interesting, however, there are probably some real downsides to this? Maybe someone else can highlight some of those.

An issue I can think of is for instance: consider a system where each time a new request comes in a new transaction is started. That situation would result in having a new `DBImpl struct` for each request. It seems that that last "but you probably only have one of these DB objects" doesn't hold. How would you tackle that issue?

Sure, if you want to compose the various methods into a single transaction, then each business-logic-implementing function needs to take a transaction object as a parameter. In that case, you don't even need something like DBImpl.

Looking at database code I have written, I never do the object thing (but was replying to an author who does). I typically have a package with functions that take transactions as an argument, like this:

https://github.com/jrockway/jsso2/blob/master/pkg/store/sess...

(If you poke around the package, methods that don't make more than one database call take a sqlx.ExtContext instead of a *sqlx.Tx. Functions that need to start their own transactions take the connection instead, though I'm not sure I'd recommend that approach because it hurts composability.)

To test it, I just run against an actual Postgres instance which is pretty easy to find. (I am not sure I would steal my database setup / teardown utilities from that project. The tests end up indented too far. A more idiomatic Go way is "app := testapp.New(); defer app.Cleanup(); tests go here"; in that codebase I picked the "testapp.Test(t, func(t testing.TB) { tests go here })" pattern. Definitely something I look for and avoid these days, but didn't in 2020 ;)

Thanks for your reply, very interesting, I didn't know about the `sqlx.ExtContext` interface yet! I will do a deeper dive into your project later, I'm hoping to find more inspiration.

Slightly unrelated to Parent:

I struggle very hard creating my own interfaces for my own programs. I haven't found any literature online teaching the _right way_ of coming up with your interface.

For example, I very often struggle with function parameters and return types: should my interface functions only take basic type and return basic types? Can my interface function take more concrete types? Should my interface function take interface types?

I'd love to be directed to a guide on how to create a good interface.

As an example, here's a recent interface declaration I wrote:

    type Checker interface {
        Check(context.Context, *object.Commit, bool) (bool, []string, error)
        Name() string
        Describe() string
    }

My program runs through a list of commits and runs "Checks" on them. These checks "pass (true)" or "fail (false)".

I want the user of my package to be in charge of the implementation details of a "Check", so I created the interface above.

Is it OK for the Check() function to take a *object.Commit? Should I instead pass a `SHA string` and let the implementer figure out how to get a *object.Commit from that string?

Looking up *object.Commit from SHA string doesn't seem like something you want to make every Checker implementation do (even if it's mostly boilerplate), so passing a higher-level object makes sense here.

What you might want to do is also define an interface for what Git calls "commit-ish"[0] objects, and use that in place of *object.Commit. Then you could pass e.g. *object.Tag in place of *object.Commit.

Think of the interface as a contract between implementations and users of the data structure. Typically, you use an interface when multiple implementations are capable of fulfilling the same contract. If there's only one implementation, you likely don't need an interface—for instance, you can ignore my suggestion about abstracting commit-ish if you're pretty sure your checker would never be called for a tag.

Interfaces are often useful when testing, because a mock implementation can fulfill that same contract. For instance, you might implement a *object.MockCommit which just uses static values instead of actually operating on a Git repository, and then pass that for your commit-ish in unit tests for various Checkers.

[0] https://git-scm.com/docs/gitglossary#Documentation/gitglossa...

I prefer to make the implementation package-private and hide it behind an interface if there's no way to make the implementation's zero value useful. Doing so prevents someone from trying to use it without calling the (mandatory) constructor.