How do you decide whether a codec package accepts io.Reader and whether it returns an io.ReadCloser, knowing other teams will implement and depend on those interfaces permanently?
answer
- narrowest thing you actually use
- accept interfaces, return concrete types
- whoever opened it, closes it
- interfaces cannot gain methods later
- the review is the last cheap moment
basics
~20 sAccept the narrowest interface the code actually uses, usually io.Reader, and return a Closer only when your package owns a resource the caller cannot release itself. Exported interfaces cannot gain methods once outside teams implement them.
solid answer
~50 sTwo separate calls sit inside this. First, what you *accept*: take `io.Reader` rather than `*os.File` or a fat `io.ReadSeekCloser`, because every method you demand narrows who can call you and rules out in-memory test doubles -- ask what the code genuinely uses, not what today's caller happens to have. Second, what you *return*: hand back a concrete type such as `*Decoder`, which you can add methods to later without breaking anyone, and expose `Close` only if your package opened something. If the caller supplied the reader, the caller closes it; a decoder that closes a stream it did not open surprises everyone. The irreversible part is any interface you *export for others to implement*: Go has no default methods, so adding a method later breaks every external implementer at compile time. Before that constituency exists you can still change it, which is exactly why the review happens now.
code
go · 22 linestype Decoder struct {
r io.Reader
}
// Borrows r: the caller opened it and the caller closes it.
func NewDecoder(r io.Reader) *Decoder { return &Decoder{r: r} }
type FileDecoder struct {
*Decoder
f *os.File
}
// Opens the file itself, so it owes the caller a Close.
func Open(name string) (*FileDecoder, error) {
f, err := os.Open(name)
if err != nil {
return nil, err
}
return &FileDecoder{Decoder: NewDecoder(f), f: f}, nil
}
func (d *FileDecoder) Close() error { return d.f.Close() }go deeper
Know the two habits behind this: take io.Reader rather than a concrete file type, and do not close a stream that somebody handed you.
Explain why a narrow parameter widens who can call you, including tests using in-memory readers, and why returning a concrete struct leaves room to add methods later.
Be able to trace ownership through a package: which constructor opened a resource, who must call Close, and what leaks or breaks when that answer is left implicit.
Own the irreversibility argument. Name what is still cheap to change and what freezes the moment other teams implement your interface, and be ready to hold a narrow surface against a consuming team asking for convenience today.
## Three decisions hiding in one question A streaming codec package exposes a surface with three distinct axes, and they have different reversibility: 1. **What the package accepts.** Widening a parameter later (from `*os.File` to `io.Reader`) is a source-compatible change for callers. 2. **What the package returns.** Returning a concrete `*Decoder` leaves room to add methods; returning an interface freezes the return surface. 3. **What the package asks other people to implement.** This is the one that cannot be undone, and it is where the review effort belongs. ## Accept the narrowest interface you actually use The Go proverb is "accept interfaces, return structs", and the operative word is *narrowest*. If your decoder only ever calls `Read`, take `io.Reader`. Taking `*os.File` means nobody can decode from a network connection, an in-memory `bytes.Reader`, a decompressing wrapper, or a fake in a unit test. Taking `io.ReadSeekCloser` when you never seek quietly excludes every non-seekable source -- sockets and pipes -- for a capability you do not use. The counter-pressure is real and should be weighed honestly: if your format genuinely needs random access, demanding `io.ReadSeeker` is correct, and pretending otherwise pushes the complexity onto callers who then have to buffer the whole stream. The test is whether the method appears in your implementation, not whether it might be convenient someday. ## Return a concrete type, and be deliberate about Close Return `*Decoder`. A concrete struct can grow methods, fields and options over time without breaking a single caller; an exported interface cannot. The exception is when callers legitimately need to substitute their own implementation -- and even then, defining that interface in the *consuming* package is usually better than exporting one from yours. The `Close` decision follows resource ownership: - **The caller opened the stream** (they passed you an `io.Reader`): you do not close it. Closing something you were merely lent breaks composition -- the caller may want to keep reading after your framing ends, or write a trailer. - **Your package opened the stream** (a convenience `Open(name string)` constructor): you must return something with `Close`, because the caller has no other handle on the file descriptor. - **You hold a resource of your own** -- a background goroutine, a pooled buffer, a flush that must happen before the data is valid -- then you owe the caller a `Close` even if you did not open the input, and its documentation must say what it releases and whether calling it twice is safe. The failure mode of getting this wrong in the permissive direction is a leaked descriptor per request; in the restrictive direction it is a `Close` that surprises the caller by shutting down a connection they still needed. Both are cheap to avoid at design time and expensive afterwards. ## The irreversible axis: interfaces others implement If your package exports an interface -- say a `Codec` that teams implement for their own formats -- you have created a contract with no escape hatch. Go has no default methods, so adding a single method later fails to compile for every external implementer. Deleting one is equally disruptive in the other direction. Interface changes are why a package that started as an internal helper becomes hard to evolve the moment three teams build on it. Practical positions to hold in review: - **Keep implementable interfaces at one or two methods.** The `io` package's whole design is this argument made at scale: one method, and everything composes. - **Prefer a struct with a function field, or an option, over an interface**, when there is only one thing the caller supplies. Adding a second option does not break anyone. - **Extend by defining a new, separate interface and type-asserting for it**, rather than by adding a method. Callers who implement the richer interface get the extra behaviour; everyone else keeps working. This is how the standard library has grown its I/O surface without breaking implementers. - **Write down what implementers must guarantee**, not merely the signatures. "Must not retain the slice", "safe for concurrent use", "Close may be called twice" are the parts nobody can infer, and the parts that cause bugs in other teams' code. ## The organisational frame The leverage is the review, before external implementations exist. Say so explicitly when you approve: an internal package can be changed in an afternoon across one repository, and the same package after three teams implement its interface needs a coordinated migration. If the shape is genuinely uncertain, the right call may be to keep the interface unexported for a release, expose only concrete types, and let real usage tell you what belongs in it -- shipping a smaller surface and widening later is nearly always cheaper than shipping a wide one and retracting. When a consuming team pushes back and asks for the fat interface because it is convenient for them today, the counter-argument is not aesthetic. It is that you are trading their convenience this quarter against everyone's inability to change the package afterwards, and that the narrow version costs them one line of adaptation.
- What changes the day an exported interface has implementations outside your repository?Adding a method stops being a refactor and becomes a breaking change: Go has no default implementations, so every external type that satisfied the interface fails to compile. Before that point you can edit all implementations in one commit; after it you need a new interface, a type assertion, and a migration other teams have to schedule.
- A consuming team wants your constructor to take io.ReadSeekCloser because that is what they already hold. How do you answer?Ask which of those methods the decoder calls. If it only reads, the wider parameter buys them nothing and costs everyone else the ability to decode from a socket or a test buffer. Passing a `*os.File` where an `io.Reader` is wanted needs no adaptation at all, so the convenience argument is close to zero.
- When would you export an interface anyway rather than a concrete type?When substitution by outside code is the actual purpose -- a plugin point, or a seam consumers must fake. Even then, keep it to one or two methods, document what implementers must guarantee beyond the signatures, and plan to extend via a second interface plus a type assertion rather than by growing the first.
saying these in an interview costs you the question
- Takes *os.File when the code only ever reads
- Exports a wide interface because it looks more flexible
- Closes a stream the caller opened and passed in
- Returns an interface, then cannot add a method later
- Assumes an exported interface can gain methods in a minor release