Re: [go-nuts] Interface embedding

[Remko Tronçon]

3. We want to be resilient when other methods are added to the 
DatastoreClient interface so that the code will still compile when that 
happens.


That was my guess too: that it's a tradeoff between safety and future 
compatibility. This allows your library users to use newer versions of the 
protobuf dependency, without forcing the library to be updated by the 
library author or the library dependency to be bumped by the user. On the 
other hand, so would embedding the interface anonymously, but maybe the 
failures are more subtle if you accidentally call a method you didn't 
account for (e.g., in this case, no headers are passed).

However in fact, there appears to be no need (*) for the datastoreClient 
type to implement pb.DatastoreClient at all


The unit tests depend on this being an interface that can be faked; I guess 
this avoids having to create your own interface for the subset of methods 
you're calling on the client. 
It would also enable library users to pass their own client, although this 
doesn't seem to be possible using the API.

thanks,
Remko

-- 
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+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/golang-nuts/7cef7f85-6760-4f42-bd24-46e495a711fen%40googlegroups.com.

Re: [go-nuts] Interface embedding

[roger peppe]

On Tue, 29 Aug 2023 at 13:35, Remko Tronçon  wrote:

> The Google Cloud Go library contains the following code (See
> https://github.com/googleapis/google-cloud-go/blob/38a040e213cc8af5b01b3afe422481493f54382f/datastore/client.go#L36
> )
>
> // datastoreClient is a wrapper for the pb.DatastoreClient
> type datastoreClient struct {
> // Embed so we still implement the DatastoreClient interface,
> // if the interface adds more methods.
> pb.DatastoreClient
> c  pb.DatastoreClient
> }
>
> func newDatastoreClient() pb.DatastoreClient {
> return { c: pb.NewDatastoreClient() }
> }
>
> func (dc *datastoreClient) Lookup() {
> // ...
>
> What could be the reason for both an anonymous and a named structure
> implementation, without initializing the anonymous one? As I understand it,
> this means the compiler no longer complains about missing methods from the
> `pb.DatastoreClient` interface, but it would crash with a nil pointer error
> when they are called?
>
> thanks,
> Remko
>
> --
> 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+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/golang-nuts/dfe1b583-5d79-4264-9a04-751ced59b73fn%40googlegroups.com
> 
> .
>

The comment gives at least one reason, but I'm not sure I'm entirely
convinced it's worthwhile.

I guess the reasoning could go something like:
1. We want to wrap the datastore client with our own methods, providing
specific functionality.
2. We definitely don't want any of the wrapper client's methods to be
called - we'd prefer to panic in that case.
3. We want to be resilient when other methods are added to the
DatastoreClient interface so that the code will still compile when that
happens.

Points 2 would argue against just embedding the wrapped client directly:
suppose that technique was used and a new method was added - the embedding
would cause the new method to be called on the wrapped client. In this
particular scenario, one could argue that the wrapper type is unexported,
so the package has full control of which methods are called, so this is
just defensive programming.

However in fact, there appears to be no need (*) for the datastoreClient
type to implement pb.DatastoreClient at all - IMHO it would all be simpler
and more obvious if the code was just using that type directly, perhaps
defining its own interface for testing purposes.

(*) a fact which is easy to check by removing the embedded interface,
renaming a method and checking that it still compiles.

-- 
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+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/golang-nuts/CAJhgachYk%2BPnJZiEiM4sRdxr-e%3DK_ZzZopefcotBTKYXig1qmw%40mail.gmail.com.

[go-nuts] Interface embedding

[Remko Tronçon]

The Google Cloud Go library contains the following code (See 
https://github.com/googleapis/google-cloud-go/blob/38a040e213cc8af5b01b3afe422481493f54382f/datastore/client.go#L36
 
)

// datastoreClient is a wrapper for the pb.DatastoreClient
type datastoreClient struct {
// Embed so we still implement the DatastoreClient interface,
// if the interface adds more methods.
pb.DatastoreClient
c  pb.DatastoreClient
}

func newDatastoreClient() pb.DatastoreClient {
return { c: pb.NewDatastoreClient() }
}

func (dc *datastoreClient) Lookup() {
// ...

What could be the reason for both an anonymous and a named structure 
implementation, without initializing the anonymous one? As I understand it, 
this means the compiler no longer complains about missing methods from the 
`pb.DatastoreClient` interface, but it would crash with a nil pointer error 
when they are called?

thanks,
Remko

-- 
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+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/golang-nuts/dfe1b583-5d79-4264-9a04-751ced59b73fn%40googlegroups.com.