trjhtr

I am in the process of learning go and am coming from a PHP, JS, and Nodejs background.

I created a package that is a client which connects to a socket server and processes the data received from the server. I am using a callback pattern and am wondering if this is frowned upon in the Go community. Here are the relevant code snippets:

// SocketClient allows handles the socket connection to a server
type SocketClient struct 
 Host string
 Path string
 conn *websocket.Conn


// SocketResponse function callback for data from socket
type SocketResponse func(res byte, err error)

// Connect create the connection with the host
func (sc SocketClient) Connect(cb SocketResponse) 
 interrupt := make(chan os.Signal, 1)
 signal.Notify(interrupt, os.Interrupt)

 u := url.URLScheme: "wss", Host: sc.Host, Path: sc.Path

 var err error
 sc.conn, _, err = websocket.DefaultDialer.Dial(u.String(), nil)

 if err != nil 
 cb(nil, err)
 

 defer sc.conn.Close()

 done := make(chan struct)

 // anonymous function call
 go func() 
 defer sc.conn.Close()
 defer close(done)
 for 
 _, message, err := sc.conn.ReadMessage()
 if err != nil 
 cb(nil, err)
 
 cb(message, nil)
 
 ()

 // ...

And then in another file I actually initialize SocketClient and call Connect...

sc.Connect(func(res byte, err error) 
 // handle the response
)

I am using a callback pattern and am wondering if this is frowned upon in the Go community.

"callback pattern" shouldn't be frowned upon (they are used in "first-class functions" for instance: https://blog.golang.org/first-class-functions-in-go-and-new-go https://dave.cheney.net/2016/11/13/do-not-fear-first-class-functions)

However regarding your code, I don't think that it looks like idiomatic go code.

Don't just check errors, handle them gracefully

sc.conn, _, err = websocket.DefaultDialer.Dial(u.String(), nil)

if err != nil 
 cb(nil, err)

You pass the err to your callback, but you then continue inside the Connect function: if you couldn't dial, why do you continue?

Never start a goroutine without knowing how it will stop

go func() 
 defer sc.conn.Close()
 defer close(done)
 for 
 _, message, err := sc.conn.ReadMessage()
 if err != nil 
 cb(nil, err)
 
 cb(message, nil)
 
()

The inner for loop will never stop (so will your goroutine).

One possibility would be to change it like this:

// SocketResponse function callback for data from socket
type SocketResponse func(res byte)

// Connect create the connection with the host
func (sc SocketClient) Connect(cb SocketResponse) error 
 u := url.URLScheme: "wss", Host: sc.Host, Path: sc.Path

 var err error
 sc.conn, _, err = websocket.DefaultDialer.Dial(u.String(), nil)

 if err != nil 
 return err
 

 defer sc.conn.Close()
 for 
 _, message, err := sc.conn.ReadMessage()
 if err != nil 
 return err
 
 cb(message)
 

Some properties of this code :

• on dial error, it stops


• it reads all the messages until one error is encountered


• it blocks (but go sc.Connect() is easy to write)

Some possible evolutions:

• to continue even in case of errors, you could return a channel of errors (and send all errors encountered)


• also use a channel to report the received messages


• to control when the Connect method should stop, you could use a context or split your method in 3 parts: Connect, ReadMessage & Close