RFC: function or interface?

[Adam Pritchard]

I’m working on a library to help get the “real” client IP from HTTP requests:
https://github.com/realclientip/realclientip-go
https://pkg.go.dev/github.com/realclientip/realclientip-go

Right now the “strategies” are like:

type Strategy func(headers http.Header, remoteAddr string) string
...
clientIPStrategy, err := realclientip.RightmostTrustedCountStrategy("X-Forwarded-For", 1)
...
clientIP := clientIPStrategy(req.Header, req.RemoteAddr)

So, functions matching a signature are created that process the input.

But I keep wondering: Should I be returning types (structs) adhering to an interface instead?

I’m starting to think I should, but I can’t think of what difference it would make.

Any feedback would be appreciated.

Adam Pritchard

​

[burak serdar]

One option is to return interfaces, but at the same time define a function type that implements that interface:

type Strategy interface {

   GetIP(headers http.Header, remoteAddr string) string

}

type StrategyFunc func(headers http.Header, remoteAddr string) string

func (s StrategyFunc) GetIP(headers http.Header, remoteAddr string) string {return s(headers,remoteAddr)}

This way, you can use functions of type StrategyFunc as well as interfaces of type Strategy when you create such strategies. The users of the library would always see the interface.

This is similar to how the handlers are done in the http library.

 

Any feedback would be appreciated.

Adam Pritchard

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/20a8eac9-98b2-4af5-87f3-e811d5a97e3cn%40googlegroups.com.

[Martin Atkins]

From the perspective of "what difference would it make" the only thing that comes immediately to my mind is that if you return an interface type that is designed so that only types in your own package can or need to implement it then you could potentially add other methods to it in future without breaking compatibility with existing callers. I can't guess what the future might hold here, so I think only you can make a best guess about how this API might evolve in future. (If it were an interface intended to be implemented by others then that argument would not hold, because extending the interface would break the existing implementations.)

However, for me it feels more important to consider what it looks like at the callsite of the result. Something that seems "off" to me on first read of your example is you named your function variable clientIPStrategy, which is a "nouny" name rather than a "verby" name. I might instead have called it getClientIP, so that the final call would be:

    clientIP := getClientIP(req.Header, req.RemoteAddr)

Since you can't control what a caller will name the result, it might help make the call site more readable to include a method with a more "verby" name:

    clientIP := clientIPStrategy.ClientIP(req.header, req.RemoteAddr)

(note: "client IP" isn't technically a verb of course, but Effective Go calls for us to leave the "get" verb implied here, so I'm settling for just picking a noun that directly describes what this function returns.)

However, that doesn't necessarily require an interface. You could define a method on your existing Strategy type to get that effect:

func (s Strategy) ClientIP(headers http.Header, remoteAddr string) string {

    return s(headers, remoteAddr)

}

So this alone doesn't seem like a justification to use an interface type. If this were an interface type, I suppose it would conventionally be named something like ClientIPer rather than Strategy, which feels idiosyncratic and leaves me to wonder what the RightmostTrustedCountStrategy function would be renamed to in that case.

A typical reason for defining an interface type is so that other packages can define implementations of it and pass them into the package that defined the interface. In your case you are returning a value so presumably all of the implementations of this interface would be inside your package anyway, and it would not be useful for another package to define another implementation because that other package would not be able to coerce this function into returning an instance of it.

With all of this said then, I don't find it compelling to define an interface type for this situation: the function type you defined is sufficient for what's needed here, and I prefer to do the simplest thing that can work. I might still try to finesse the callsite by fiddling with the names or by adding that optional method, along the lines I showed above, but that's a separate concern from what type this function should return.

YMMV, of course. This is a pretty subjective question! Hopefully even if you don't agree with my conclusion the journey I took to get there is still useful to you.

[Adam Pritchard]

Thank you both for your responses.

I’m going to switch to returning structs that implement an unexported interface. I don’t have a super convincing justification, but here are the reasons…

1: It’s unexported because it’s not expected to be implemented externally and passed in. But if a caller wants to implement their own (and still use Must or ChainStategies, for example), then they can define the interface themselves — I’ll have comment indicating this. (This unexported interface thing is used by pkg/errors and I’ve read reference to it elsewhere, but I can’t find where now.)

2: Why I’m concerned with implementing a particular interface or function signature at all, when I could just return any old thing: I would like to think that some users of the library would make the strategy a runtime decision based on configuration. So it would be nice if their code paths don’t care which strategy it is.

3: If the caller wants just a function, they can easily get with a bound method. Something like:

getClientIP := realclientip.NewSingleIPHeaderStrategy("X-Real-IP").ClientIP

Not lovely, but it works the same.

4: As Martin said, an interface means I could “potentially add other methods to it in future without breaking compatibility”. Now, I really, specifically want this to be very simple, slim library (I really do want it to be reimplemented in other languages), but I can imagine, say, collecting stats on calls and failures (empty string returns). Then adding a String() method that prints those stats and/or the strategy type and configuration (I like logging such things). (Yes, the library user could also just add a wrapper that does that.)

I haven’t start making the changes yet, so it could all feel like garbage — we’ll see. I’m also not yet settled on the names of things — I’ll pay more attention to the verb-iness/noun-iness.

Thanks again.

Adam

​