clickhouse-go
clickhouse-go copied to clipboard
NOP/Dummy connection
Is your feature request related to a problem? Please describe. Adding nil checks in every location adds verbosity to code
Describe the solution you'd like A dummy or NOP connection to serve the purpose of a drop in replacement for a real connection
Describe alternatives you've considered Currently adding nil checks every time a connection could be used
Additional context N/A
@GTB3NW can you provide an example of usage that requires these nil checks? Nil checks area common go pattern but appreciate it might be useful to be able to mock driver in some scenarios.
For sure @gingerwizard, it's a fairly idiomatic check in go, however in terms of SDK usage I think a no-op driver would be ideal to reduce nil check bloats (if anything, nil check bloat is one of the main criticisms of the language).
Two examples:
if n.chConn != nil {
ctx1, cancel1 := context.WithTimeout(context.Background(), 1*time.Second)
defer cancel1()
if err := n.chConn.Ping(ctx1); err != nil {
return err
}
}
// more code here
Okay, that's not too bad, but it serves as a basic example, the second example might show where it bloats the code..
var batch driver.Batch
var err error
if n.chConn != nil {
batch, err = n.chConn.PrepareBatch(context.Background(), MY_CONST_DSL)
if err != nil {
return err
}
}
for _, foo := range bar {
// do some work which can't be behind the nil check, for example stuff which is unit tested
if n.chConn != nil {
if err := batch.AppendStruct(&types.MyEvent{
EventID: uuid.New(),
}); err != nil {
return err
}
}
}
if n.chConn != nil {
if err := batch.Send(); err != nil {
return err
}
}
sorry for the formatting, I've cobbled it together
I dont quite understand why you can't just implement the Conn interface with your own dummy connection? it's not the responsibility of the driver to provide dummy test connections.
I dont quite understand why you can't just implement the Conn interface with your own dummy connection?
That's an option, honestly, not sure why I didn't think about that
It's not the responsibility of the driver to provide dummy test connections.
I'd disagree but I don't want to come off as a naggy user lol. NOP implementations are really common for situations where you're not looking to test the third party library. I'd argue strongly that the less friction involved for people to adopt your library, the more people will pick it up.
Thanks for getting back to me, please don't take the above as criticism because I do appreciate the library it's great. If I get a moment I will PR a NOP connection with your blessing.
@GTB3NW np at all. Happy to accept such a PR if its cleanly seperated e.g. under ./test-lib