Generate an interface that both the PGClient and TxPGClient comply with
NickPPC opened this issue · 8 comments
Generating an interface would make a lot of things easier regarding integration with other library/framework. A non negligible benefit would be that it is then easy to generate mocks for testing.
PGClient and TxPGClient already have the same public method except for the one related to the transaction (start, commit, rollback) those could simply return an error
That seems like a good idea, though I'm pretty against mocking the database :)
WRT the transaction management stuff, I think it would be better to just leave those out of the interface if they are not actually common to the types. I think having to downcast is a better UX than having a method panic on you.
Because you marked it "good first issue" I'll mention some general thoughts about implementation. I think it would make sense to add a new gen_interface.go
file in the gen
package that follows the same pattern as all the other gen_*
files in that package. The template could just stamp out an type IPGClient interface { ... }
. (IPGClient
is a horrible name, but I'm struggling to think of a good name that doesn't clash with PGClient
).
I think PGQueries
would be a much better interface name. It encapsulates the fact that it contains the methods common to PGClient and TxPGClient reasonably well, given that the methods common to the two are generally concerned with talking to the database.
An alternative to avoid the transaction issue would be to create one interface for the common stuff and one for each that inherit/include/ the common one
Generating a interface to wrap each struct makes me a little grumpy because it make me worried that it will encourage excessive mocking, but I can see why we would want to be able to generate OpenTracing wrappers and whatnot around the PGClient. Also, there is clearly a mock-happy subculture in go and pggen should probably be usable with that style of programming.
@alew97 if you do start digging into this and want pointers about where to start, I'm happy to provide them.
@NickPPC I think I want to keep separate interfaces for PGClient and TxPGClient out of pggen
core. The extra methods that PGClient
has are a short, static list, so it is easy for client code to define such interfaces, whereas it is impossible to define the intersection interface in client code in such a way that things won't break when you add new tables or queries to your config file.
I think keeping the extra interfaces out is better for a number of reasons:
- It is not needed to achieve the desired functionality, so leaving it out gives us a smaller API surface to maintain
- Go style encourages defining interfaces at the point of use rather than where the type is defined.
- I think having an interface with exactly one implementer is a pretty big code smell, as is mocking the database. I don't want pggen to generate code that encourages people to write code that follows what is, in my view, a poor style. Of course I understand that views differ on style questions, but this choice won't preclude people from mocking the database, it will just mean that they have to write four or five extra lines in order to do so.
I think having just the common one with the Get/List/Insert... and query defined method is good enough in 95% of cases