3 ms·

Fair criticism. I'm the author, and I hate using interfaces like this, but didn't comment on it probably because I got used to it for this kind of thing. The o

by henvic 5y ago

Fair criticism. I'm the author, and I hate using interfaces like this, but didn't comment on it probably because I got used to it for this kind of thing. The only reason why I defined inventory.DB (https://github.com/henvic/pgxtutorial/blob/main/internal/inventory/service.go#L18-L50 https://github.com/henvic/pgxtutorial/blob/main/internal/inv...) is to be able to test. Otherwise, I'd have skipped it. I haven't read your article yet, but will do so later and get back to this subject and tell my opinion by the end of the day (on vacation, and need to hurry to catch the train!).

jrockway 5y ago
Testing is good, but probably not a good enough reason to add unbounded ongoing maintenance costs. The problem is that putting everything in one interfaces forces you to implement the test version of DB every time you add real functionality to the real implementation. That gets pretty expensive in terms of time, and forces you to do work that's not necessary. (You could always test using the real implementation, and that's the approach I'd recommend starting with. When it gets too slow, look for an alternative.) My blog post covers what unit testing is like with your interface, with a consumer-defined interface, and just putting some test data fields in the DBImpl struct. The latter is my preferred approach. It took me a long time to get there. I was working at Google writing some filesystem access code and wanted to test it. I designed it as interfaces so I could have a real implementation and a test implementation, and a member of the Go team reviewed the code and told me it was absolutely inappropriate to use interfaces there. I disagreed at the time, but I've slowly come to terms with this style. Separating out production and testing is "clean", but that cleanliness comes at the cost of a mental abstraction burden (every time you read the code you're confused about where the meat lives) and unbounded future maintenance costs (every time you want to add a new feature, you have to edit two files). I don't think having a mock framework do codegen to make generic mocks is that useful. Just keep the exact behavior close to the test. A working test harness for a particular unit is something small enough to write out in an HN comment, and keeping the test code close to the test (and not wrapped up in some complicated framework) makes it much easier to enhance and debug the tests. (I've had Mockery forced upon me before, and I'm miserable every time I have to interact with it. Have to get the right version. Have to update an interface after writing the real implementation. Have to regenerate the mocks. Then I have to refer to the docs to do even the simplest tests, and those tests end up not doing what I want (instead producing something useless like "assert that function was called with the right args"). It's a chore that isn't worth the cost.) I think the underlying psychology behind the mega-interfaces is probably that we come up with an idea for what the interface should look like, and want to write it down somewhere. Then we can disengage the brain and start filling out the method bodies. That is not a good enough reason to add a layer of abstraction to the code; you can always type out 'func RealImplementation() { panic("write this") }' if that's the real reason. My view shifted when I was trying to use the OpenTelemetry library (circa v0.6.0) to emit Prometheus metrics. I wanted to set the histogram buckets on a per-metric basis. I chased 100 layers of indirection (every function took and returned an interface, so you'd need to have like 5 code windows open to debug the flow of data between the interface level and the implementation level), and never figured out how to do it. I think I determined it wasn't possible. It was at that moment that I decided "wait, I think everyone is using interfaces wrong", and then found CodeReviewComments telling me "yup, it's wrong".
- henvic 5y ago
  I read your article and it has good points. I've added a section pointing to it just after I talked about interfaces, so other people can read it too.
  - jrockway 5y ago
    Thanks for giving it a readthrough! Just wanted to mention that even though I'm critical about this one thing, your article as a whole is superb. If people start their Go projects using pgx, tern, and have tests for their database functionality, they're going to be set up for a lot of success. The interface thing is just a minor annoyance I have because I see so many people doing it. (I'm sure my past self has done it more times than I'd like to admit.)
- jamesrr39 5y ago
  For testing I wrote https://github.com/jamesrr39/go-mockgen-tool https://github.com/jamesrr39/go-mockgen-tool to generate mock implementations of a given interface. It's a different approach from the normal reflect and interface{}... heavy libraries. Might be interesting!
  - pqb 5y ago
    Looks interesting! Reminds me impl[0], which comes bundled with `vscode-golang`. [0]: https://github.com/josharian/impl https://github.com/josharian/impl
  - jrockway 5y ago
    Quite reasonable! I think if you keep your interfaces small, you might not need it, but if you do have a couple of beefy interfaces, then this is a lot more pleasant than using a dedicated mock framework. A function is a great way to mock a function -- you can do anything! ;)

by henvic 5y ago

jrockway 5y ago

Testing is good, but probably not a good enough reason to add unbounded ongoing maintenance costs. The problem is that putting everything in one interfaces forces you to implement the test version of DB every time you add real functionality to the real implementation. That gets pretty expensive in terms of time, and forces you to do work that's not necessary. (You could always test using the real implementation, and that's the approach I'd recommend starting with. When it gets too slow, look for an alternative.) My blog post covers what unit testing is like with your interface, with a consumer-defined interface, and just putting some test data fields in the DBImpl struct. The latter is my preferred approach. It took me a long time to get there. I was working at Google writing some filesystem access code and wanted to test it. I designed it as interfaces so I could have a real implementation and a test implementation, and a member of the Go team reviewed the code and told me it was absolutely inappropriate to use interfaces there. I disagreed at the time, but I've slowly come to terms with this style. Separating out production and testing is "clean", but that cleanliness comes at the cost of a mental abstraction burden (every time you read the code you're confused about where the meat lives) and unbounded future maintenance costs (every time you want to add a new feature, you have to edit two files). I don't think having a mock framework do codegen to make generic mocks is that useful. Just keep the exact behavior close to the test. A working test harness for a particular unit is something small enough to write out in an HN comment, and keeping the test code close to the test (and not wrapped up in some complicated framework) makes it much easier to enhance and debug the tests. (I've had Mockery forced upon me before, and I'm miserable every time I have to interact with it. Have to get the right version. Have to update an interface after writing the real implementation. Have to regenerate the mocks. Then I have to refer to the docs to do even the simplest tests, and those tests end up not doing what I want (instead producing something useless like "assert that function was called with the right args"). It's a chore that isn't worth the cost.) I think the underlying psychology behind the mega-interfaces is probably that we come up with an idea for what the interface should look like, and want to write it down somewhere. Then we can disengage the brain and start filling out the method bodies. That is not a good enough reason to add a layer of abstraction to the code; you can always type out 'func RealImplementation() { panic("write this") }' if that's the real reason. My view shifted when I was trying to use the OpenTelemetry library (circa v0.6.0) to emit Prometheus metrics. I wanted to set the histogram buckets on a per-metric basis. I chased 100 layers of indirection (every function took and returned an interface, so you'd need to have like 5 code windows open to debug the flow of data between the interface level and the implementation level), and never figured out how to do it. I think I determined it wasn't possible. It was at that moment that I decided "wait, I think everyone is using interfaces wrong", and then found CodeReviewComments telling me "yup, it's wrong".

henvic 5y ago
I read your article and it has good points. I've added a section pointing to it just after I talked about interfaces, so other people can read it too.
- jrockway 5y ago
  Thanks for giving it a readthrough! Just wanted to mention that even though I'm critical about this one thing, your article as a whole is superb. If people start their Go projects using pgx, tern, and have tests for their database functionality, they're going to be set up for a lot of success. The interface thing is just a minor annoyance I have because I see so many people doing it. (I'm sure my past self has done it more times than I'd like to admit.)
jamesrr39 5y ago
For testing I wrote https://github.com/jamesrr39/go-mockgen-tool https://github.com/jamesrr39/go-mockgen-tool to generate mock implementations of a given interface. It's a different approach from the normal reflect and interface{}... heavy libraries. Might be interesting!
- pqb 5y ago
  Looks interesting! Reminds me impl[0], which comes bundled with `vscode-golang`. [0]: https://github.com/josharian/impl https://github.com/josharian/impl
- jrockway 5y ago
  Quite reasonable! I think if you keep your interfaces small, you might not need it, but if you do have a couple of beefy interfaces, then this is a lot more pleasant than using a dedicated mock framework. A function is a great way to mock a function -- you can do anything! ;)