chengxilo opened a new pull request, #1916:
URL: https://github.com/apache/iggy/pull/1916
# Some explanation
## What I did
1. refactor logic for building new Go SDK client using builder pattern
2. I changed some function to make them align with Rust SDK ( not only their
name, I also edited part of their output)
3. I updated the tests.
## Why we need to refactor it.
( I copied the message I sent in discord a few days ago here)
Since currently, the golang sdk is very out dated and when we create a
client the
in rust when we create client:
```rust
let client = IggyClientBuilder::new()
.with_tcp()
.with_server_address(get_tcp_server_addr())
.build()?;
```
While in golang we create client:
```go
factory := iggy.IggyClientFactory{}
msgStream, _ := factory.CreateMessageStream(iggcon.IggyConfiguration{
BaseAddress: "vm:8090",
Protocol: "tcp",
})
```
Since the Rust SDK is considered the primary or canonical implementation, it
makes sense for other language SDKs to follow its design principles closely —
including using the same repository name and similar function names. Clearly,
the current Go SDK does not meet this requirement: its function names don’t
clearly describe their purpose, and the overall design pattern feels less
elegant.
## My solution
So what I did is to read the rust sdk source code and ... almost directly
ported its logic to Go. I want to explain why I decided not to use the option
pattern discussed earlier with @haze518, even though the option pattern is
commonly considered best practice in Go. At first, I choose to follow the
example of https://github.com/nats-io/nats.go/blob/main/nats.go#L867 . It's
elegent, but it would not work so well in our use case, since we need to
support tcp/http/quic.
It's because three different protocol have three different client.
Here is a simplified example showing the potential issue:
```
package main
type Options struct {
protocol string
aTcpOpt bool // a TCP-specific option
aHttpOpt bool // a HTTP-specific option
}
type Option func(*Options)
func WithTcp(opts *Options) {
opts.protocol = "tcp"
}
func WithHttp(opts *Options) {
opts.protocol = "http"
}
func WithATcpOpt(opts *Options) {
opts.aTcpOpt = true
}
func WithAHttpOpt(opts *Options) {
opts.aHttpOpt = true
}
func New(opts ...Option) {
options := &Options{
protocol: "tcp",
aTcpOpt: false,
aHttpOpt: false,
}
for _, opt := range opts {
opt(options)
}
// ... build the client with option
}
func main() {
New(WithTcp, WithAHttpOpt, WithATcpOpt) // user may add a http option
when they are using tcp
}
```
So, I think it is not a good choice as I don't know how to solve this
problem while using option pattern. But if we use builder pattern, it allows us
to control and constrain users’ choices more effectively, preventing invalid
combinations and leading to a cleaner, safer API.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
