Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wiki: CodeReviewComments change #33228

Open
eslam-mahmoud opened this issue Jul 22, 2019 · 4 comments

Comments

@eslam-mahmoud
Copy link

commented Jul 22, 2019

Hello,

I suggest an example to be added to explain why interface constructor should return the struct not the interface type itself

The edit to be added on CodeReviewComments #interfaces

Go interfaces generally belong in the package that uses values of the interface type, not the package that implements those values. The implementing package should return concrete (usually pointer or struct) types: that way, new methods can be added to implementations without requiring extensive refactoring, **and this will enable the struct to implement more than one interface and be accepted/evaluated to all of them in different functions**

The code example
https://play.golang.org/p/zZU0rFzjuEm

// DONOT DO THIS
package main
import ("io")
type Thinger interface { 
	Thing() bool 
}
type defaultThinger struct{  }
func (t defaultThinger) Thing() bool { 
	return true 
}
func (t defaultThinger) Read(p []byte) (n int, err error) {
   	return 0, nil
}  	
func NewThinger() Thinger { 
	return defaultThinger{  } 
}
func main() {
	testThinger(NewThinger())
	// will return cannot use NewThinger() (type Thinger) as type io.Reader in argument to testReader: Thinger does not implement io.Reader (missing Read method)
	// if NewThinger() return defaultThinger it will work
	testReader(NewThinger())
}
func testReader(r io.Reader) {}
func testThinger(t Thinger) {}
@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Jul 22, 2019

Honestly, that seems like a minor point to me, compared to the existing point.

CC @bcmills

@eslam-mahmoud

This comment has been minimized.

Copy link
Author

commented Jul 22, 2019

The point is to clarify the why with an example of what can go wrong

@bcmills

This comment has been minimized.

Copy link
Member

commented Jul 22, 2019

Implementing a second interface seems like a special case of the general problem of wanting to access methods beyond the first interface.

In particular, it would seem to suggest that you could instead return an interface that composes together the various other interfaces that you intend to implement. But that would still have the same problems as returning an interface in general: it still would not allow you to add methods beyond the original interface, and still would not allow clarifying documentation on the method implementations.

@eslam-mahmoud

This comment has been minimized.

Copy link
Author

commented Jul 29, 2019

I agree with you, and because I faced that case I tried to add it for everyone to be one more point listed explaining why not to return the interface

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.