Preferences for Techniques for dealing with changing levels of encapsulation

Reg · May 18, 2020, 1:38am

I’ve noticed as I get more into go programming something that comes up quite often is a small code change could cause me having to make many code changes just because of scope of a variable encapsulation change in a function, very often with a local err variable. For example, let’s say I have the following code snippet:

func A(ic *b) {
	...
	...
	go func() {
		logger.Debug(`ListenAndServeTLS`, 1)
		err := httpServer.ListenAndServeTLS(cert, key)
		if err == http.ErrServerClosed {
			logger.Debug(err, 2)
		} else {
			logger.Error(err)
		}
	}()
}

Now suppose I need to add a few lines so it becomes:

func A(ic *b) {
		...
	_, err := os.Stat(cert)
	if err != nil {
			...
	}
	...
	go func() {
		logger.Debug(`ListenAndServeTLS`, 1)
		err := httpServer.ListenAndServeTLS(cert, key)
		if err == http.ErrServerClosed {
			logger.Debug(err, 2)
		} else {
			logger.Error(err)
		}
	}()
}

So at this point there are few things we can do to deal with the scoping change of err. We could change err := ... to err = ... but if there are places with other variables being returned with err then you have to change the variable declaration for them which in a longer function can be annoying. Alternatively you could just put the new lines in a closure like:

{
	_, err := os.Stat(cert)
	if err != nil {
			...
	}
}

which takes the pain out of changing lots of other places in the code. I don’t think there is a one right answer as to what to do in these situations but I would like to hear from the experienced go programmers on how they mostly deal with this occurrence. Perhaps you just write in a way that this never comes up or perhaps it’s just one of the quirks of the language and we just deal with as best we can each time it happens or perhaps you have honed the best solutions through experience and wouldn’t mind sharing…

Your thoughts much appreciated.

bklimczak · May 18, 2020, 5:08am

if it’s a pain to you, you can try using this techique

if err := someFunc(); err != nil {
  // do something
}

Thanks to this, err is visible only in the if statement so no collisions should occur.

Reg · May 18, 2020, 5:19am

That looks like the way to go, thanks.

skillian · May 18, 2020, 12:05pm

I don’t understand. Why do you have to do anything just because err is defined earlier?

bklimczak · May 18, 2020, 12:50pm

he has to change err := someCode() to err = someCode(). Not a big thing:)

skillian · May 18, 2020, 1:56pm

Why does he have to do this? What’s wrong with just declaring it twice? They’re already in different scopes.

Reg · May 19, 2020, 2:38am

@skillian As soon as I add the lines of code then the IDE tells me that the lines like err := httpServer.ListenAndServeTLS(cert, key) are invalid so I have to take the colon out to make it valid so the line becomes err = httpServer.ListenAndServeTLS(cert, key).

In a short function, no big deal.
In a function with only err being returned, no big deal.

However, in a longer function where many times multiple variables are being return, consider if the line was like this instead, err, a, b := httpServer.ListenAndServeTLS(cert, key). Now taking the colon out also means I have to add in a declaration for a & b. Again, if it’s just once no big deal. However, if in the routine that means many such changes and you notice this happen often, then you might start to wonder if there is a better way.

@bklimczak hit the nail on the head as by doing:

if err := someFunc(); err != nil {
  // do something
}

instead of:

err := someFunc()
if err != nil {
  // do something
}

the problem goes away which to me confirms my intuition in that there was something I was overlooking, a rookie mistake if you will, hence, asking this question.

I hope that clears it up… well, at least as clear as mud now anyway.

skillian · May 19, 2020, 9:24am

Clear as mud I’m still confused. I took your example, put it into the Go playground and then made some changes to make it build: https://play.golang.org/p/H_bhNzMGHOp

In this example, you should not change = to := where you’re defining err. If you change line 23 from:

err := doSomething2() //httpServer.ListenAndServeTLS(cert, key)

to

err = doSomething2() //httpServer.ListenAndServeTLS(cert, key)

You create slightly less efficient code and risk introducing a race condition. err in your goroutine func is no longer a new variable with the same name as err from the outer scope, it now is the err variable from the outer scope. If you start any other goroutines, they might clobber that result: https://play.golang.org/p/Iqk_iEsXYkP

In my example, I used a sync.WaitGroup to make sure it always fails by returning the inner error, but if you instead performed I/O or made large allocations, etc. from the outer A function, it could suspend A, then run the inner goroutine, assign to A's err variable, and then resume A.

If your example is more like this: https://play.golang.org/p/FgQ045bnpvQ Then I understand your point but the compiler will complain at you for the one line where you have to change from err := ... to err = ... It’s not like you need to make the change in more place than one: https://play.golang.org/p/_dkbjZ6oKUR

Note also that if you’re not ignoring the return values, then you don’t need to change := to = at all: https://play.golang.org/p/iqj7hQXnYMi

bklimczak · May 19, 2020, 10:52am

I think that those problems come from another one - IMO those functions are too big. The first example can be refactored to

func A(ic *b) {
		...
	_, err := os.Stat(cert)
	if err != nil {
			...
	}
	...
	go run()
}

func run() {
		logger.Debug(`ListenAndServeTLS`, 1)
		err := httpServer.ListenAndServeTLS(cert, key)

		if err == http.ErrServerClosed {
			logger.Debug(err, 2)
		} else {
			logger.Error(err)
		}
}

and the problem disappears

skillian · May 19, 2020, 1:36pm

I’m still missing something. What, exactly, is the problem? The first example declares err in two scopes. That’s OK. No error. What is the problem that we’re trying to solve?

bklimczak · May 19, 2020, 1:55pm

From what I understood from the original question, the problem is a situation that you declare err somewhere deeper in the function (take a look at the very first code snippet) and you add more code before calling the anonymous func. Very often, you have to handle the error as well, so you create another if err != nil { but… but in the code, you already have (in the same scope) declared err variable. So what you have to do is changing err := something() to err = something().

It’s not an issue for me, but as you can see, some people find it problematic. That’s why I suggested to split the code and use

if err := someFunc(); err != nil {
  // do something
}

Reg · May 19, 2020, 10:07pm

@skillian I know what’s going on now. Your code in https://play.golang.org/p/iqj7hQXnYMi works in playground but the err on lines 17, 21, 25… are showing as problematic in my IDE for err until I take the colon off of those statements.

My IDE is highlighting things as a problem when the go compiler would have accepted them. I’ll have to look into it but I guess it’s some sort of hint – almost certainly warning me that I redeclared a variable.

This is, was, a problem of dealing with multiple new things at a time… go + new IDE (Goland, I’m not familiar with Jet Brains products yet) and me not realizing the problem was my misinterpretation of the IDE’s color change for the subsequent err variables.

Thank you for your persistence.

Jeff_Kayser · May 25, 2020, 4:23am

When I was first learning Go, I ran into problem with inner-scoped variables shadowing another variable with the same name in an outer scope. My initial solution was:

define all your variables at the top of the function.
Do not use “:=”, EVER. Use “=” instead.
It’s a pain, but you will never have to worry about the problem again.

Now, that I know about variable shadowing, and can look for it, I use “:=” a little more, but still sparingly.

system · August 23, 2020, 4:23am

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