Skip to content
This repository has been archived by the owner on May 29, 2018. It is now read-only.

MockPostsService setup #2

Open
ezbercih opened this issue Jul 24, 2014 · 6 comments
Open

MockPostsService setup #2

ezbercih opened this issue Jul 24, 2014 · 6 comments

Comments

@ezbercih
Copy link

Could you elaborate on the design of the MockPostsService in posts.go and decisions around how it is setup? To be more specific:

  • Why do we need the following line?
    var _ PostsService = &MockPostsService{}
  • Is it a common practice to create a shell type whose functions (that closely mimic the interface) can be swapped depending on the test and interface methods call them? Why not create separate mock types that implement the interface for different tests?
  • Lastly, what is the guidance around returning a slice of object pointers ([]*Post) vs. a slice of objects ([]Post)? Is there a significant benefit?

I see you got inspired quite a bit from https://github.com/google/go-github when designing the client and options interface ideas, with a departure from implementing services as interfaces with types that implement them instead of directly creating them as types. I am going back and forth on this on my implementation but leaning towards the interface approach for better testing.

And I just wanted to say this is an awesome project for hashing out best practices. I really appreciate the effort you guys put in it, as I am working on an SDK for a set of REST APIs and it helps to validate some of the decisions I made.

@beyang
Copy link
Member

beyang commented Jul 24, 2014

Answering questions point by point:

  • var _ PostsService = &MockPostsService{} ensures that MockPostsService actually implements the PostsService interface (otherwise, you'll get a compile-time error)
  • Creating a general mock type with stub functions that can be overwritten saves some boilerplate over writing ad-hoc mock types for every test function. For example, a given unit test might only test the Get() function and not need to specify functionality for List() or Submit(). If you were creating a new mock type for each test, then each mock type would have to implement all the functions in the interface, even if they aren't relevant to the functionality being tested.
  • There is probably a slight perf benefit when you're assigning an element of the array to another variable, e.g., var p = posts[i]. If posts is a []*Post, then only the pointer value needs to be copied; if it's a []Post, then the struct needs to be copied. Other than that, I can't think of a super compelling reason.

go-github was a big inspiration for the design of the API client. I think the main difference is that go-github is solely an API client, whereas we wanted to reuse the same code for not just the API client, but also the datastore in the server. Hence defining the interface, which gets implemented at multiple layers in the stack.

Thanks for the kind words. Look forward to the SDK, would love to explore it when it's releaseD!

@ezbercih
Copy link
Author

Thanks for the detailed explanation. Much appreciated!

I will make sure to let you know when I release the SDK.

@ezbercih
Copy link
Author

Regarding the third point, I know you mentioned arrays but in this case it looks like it is a slice and based on the comment in the second paragraph at http://golang.org/doc/faq#pass_by_value slice values behave like pointers:

Map and slice values behave like pointers: they are descriptors that contain pointers to the underlying map or slice data. Copying a map or slice value doesn't copy the data it points to.

So actually creating a slice of pointers must allocate more memory than creating a slice of values. Since slice elements are pointers to an underlying array's elements, we already have the desired effect. We would be doing double indirection (a pointer to a value in an array element vs. a pointer to a pointer in an array element that points to a value) in this case and storing an extra pointer per value. I might totally got it wrong but that's how I interpreted.

@beyang
Copy link
Member

beyang commented Jul 25, 2014

Sorry, substitute "slice" for "array" in my previous comment.

And you're right. If you do

foo := []Post{Post{ ... }, Post{ ... }, ... }
bar := foo

this doesn't actually copy the slice data. The slice itself is passed by reference.

But if you get a specific element out of a slice, it will copy that data.

foo := []Post{Post{ ... }, Post{ ... }, ... }
fooElem = foo[0]
fooElem.ID = 20394
fmt.Println(fooElem.ID)
fmt.Println(foo[0].ID)

will print different values. Whereas if it were a slice of pointers, then only the pointer would be copied. I'm not claiming this is a huge perf win. Either one probably works fine.

Also, the way the modl library deserializes data from SQL might also require an array of pointers (I can't remember specifically), but that'd be just an implementation detail.

@gbbr
Copy link

gbbr commented Nov 4, 2014

@beyang I've remember this approach you were using to verify interfaces are implemented. I've found a better one that is used in the stdlib and thought you might like to find out about it:

var _ PostsService = (*MockPostsService)(nil)

That works too. See Line 248 & 249 here http://golang.org/src/pkg/testing/testing.go

@beyang
Copy link
Member

beyang commented Nov 4, 2014

cool, thanks for sharing!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants