Is this a pattern i should avoid?

I find myself writing code like:

var (
	theCheckServers *CheckServers
	theCheckServersMu sync.Mutex
func GetCheckServers() *CheckServers {
	defer theCheckServersMu.Unlock()
	if theCheckServers == nil {
		theCheckServers = NewCheckServers()
	return theCheckServers

In order to avoid using global variables and having a singleton “instance” of the given type.

Should i avoid it?

I would rather just have return theCheckServers and let the caller decide what to do next,i.e call NewCheckServers if the function return nil

Hey @twisted1919,

If CheckServers is just an object you use only when you need it, code like this is much more preferable. No need for mutex’s and globals. Just make CheckServer a type:

package main

// CheckServer type.
type CheckServer struct {
	// ...

// NewCheckServer creates a new CheckServer type.
func NewCheckServer() *CheckServer {
	return &CheckServer{}

func main() {
	// Create a CheckServer when you need one.
	// No globals, no mutexes.
	cs := NewCheckServer()

If however, CheckServers was meant to represent a slice of CheckServer types, let me know so I can show you a better way for that too, but otherwise you should be doing something like this.

1 Like

@radovskyb / @pieterlouw - Thank you guys :wink:

Maybe sync.Once is what you are looking for. It allows running a function exactly once.

1 Like

No worries @twisted1919, I wasn’t actually taking into account with whether or not you actually needed a singleton instance or not, so if you did need one, here are 2 simple options you could use:

  1. Elaborating on what @christophberger suggested, using sync.Cond:
    You should probably however read up on using sync.Cond before using it, for example from the docs:
    If f panics, Do considers it to have returned; future calls of Do return without calling f.

  2. Using a mutex:

And here is also a simple article I just found about singleton’s in Go:

Lastly, you don’t actually need to clump everything together in types the way I’ve demonstrated above.

For example you could just chuck your mutex or sync.Once type next to your object like you originally used and it’s not an issue, but I just personally find it more clean this way.

1 Like

@radovskyb - amazing examples, thank you again. I am going to give sync.Once a try and see how that goes since i used to use the mutex approach but sync.Once seems more clean.
@christophberger - Thank you :slight_smile:

Hey @twisted1919, glad to help!

Just make sure to be careful with sync.Once because of for example what the snippet that I posted from the docs above is saying. (If f panics, Do considers it to have returned; future calls of Do return without calling f.)

Basically in this case, if the NewCheckServer() function is something that can panic or fail and won’t properly initialize a CheckServer if this happens, since the code inside of Do() has already technically been run once, even it it failed, it won’t technically run again, but if it is just a simple initialization function, then it’s simple to use :slight_smile:

1 Like

@radovskyb - Yup, thanks for the pointer, i took note about that, but it’s just simple initialization, so i should be fine :wink:

1 Like

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.