When is it okay to use package level variables?

agolangf · · 484 次点击

这是一个分享于的资源，其中的信息可能已经有所发展或是发生改变。

I often see package level variables demonized in posts. But, it's not often mentioned when it IS okay to use package level vars. This has been causing me to use a lot of structs as receivers to carry my variables, and sometimes i think it's just overkill. <hr/>**评论：** jimbishopp: <pre>When you don't limit your ability to test.</pre>_crackling: <pre>I like this rule, tyvm</pre>soapysops: <pre>I think this is the best guideline. Honestly, in general, 99% of the time the reason I'm annoyed with someone else's code is because it lacks tests. If code has good tests, it most likely doesn't have the inherent design flaws that make it hard to work with.</pre>full_metal_alch: <pre>I can see why you might think defining structs is just boilerplate, but I think it's much less painful than jumping into code that you've never seen before and having to walk through the code to see what global variables its using. Defining dependencies and configuration in a struct just makes it really easy to glance at something and understand what the code uses. It also makes testing painful when you use global variables because you could have two tests mutating or relying on it at the same time, and in general global state can be very hard to reason about. Do you have a specific example of where you think a global variable makes more sense?</pre>_crackling: <pre>I'm writing a parser right now, so for instance, look at how many times i'm typing t. and p. (tokens and parser) <pre><code>func (p *parser) block() (node *a.Node) { p.expect(t.LBRACE) node = a.NewNode(a.BLOCK, "") if p.currentToken.Typ == t.NEWLINE { p.newline() } for p.tokenIs(t.IDENTIFIER, t.VARIABLE) { if p.currentToken.Typ == t.VARIABLE { node.Add(p.variableDeclaration()...) } if p.tokenIs(t.IDENTIFIER) { node.Add(p.statement()) } if p.currentToken.Typ == t.NEWLINE { p.newline() } } if p.currentToken.Typ == t.NEWLINE { p.newline() } p.expect(t.RBRACE) return node } </code></pre></pre>jahayhurst: <pre>You're writing a parser. What if someone wants to parse two streams at once? Or 20? Probably best to make a parser object, and put things to track position in there. The way I look at it, like with the net/http library, there's a default client it uses - a global variable that holds a http.client - if you just use bare functions. That's great if you want to then do something with testing - just replace that default client and you're golden. If there could be a situation where you need multiple versions of a thing, that thing should not be global. If you just need it once, maybe a global works. I've taken to (at times) declaring all of my standard errors as globals in the package, and then heavily commenting each error - that way I can test for exactly that error, I can document in the package documentation what's causing that error, and I'll never need to change that error ever cause if I did I'll just create another.</pre>full_metal_alch: <pre>You could always just have pure functions do the "work" of the program, and have structs that manage state and call the functions with the appropriate arguments</pre>_crackling: <pre>I mean, that's a great way to do this, but unfortunately i have a few edge cases poking holes in my ability to go that route. So i was thinking maybe a couple global variables could at least clean it up quite a bit</pre>_crackling: <pre>i do like how you explain readability as a main concern over global vars though, that helped me understand. With that said, what if my package isn't very many lines? Are global vars acceptable then? (recursive decent parsers arent very large)</pre>epiris: <pre>Part of your issue could be mitigated with better naming. For example parser is qualified in receiver, so instead of currentToken at the very least curTok would be okay. But what else does your parser have besides a cur token? A look ahead or look behind? Peek() is usually a good name for a look ahead. So if all we have on p is a current token, we don’t need to clarify this in its identifier and suddenly tok emerges as a decent yet clear name. Now I only see you reference Typ directly on p, otherwise you make a function call of p.tokenIs(t). Ignoring that predicates are usually named isThing rather than thingIs - We can ask the same question if it’s not a token what else may it be? Maybe isTok would be fair and isTyp could replace field access for curTok. I’m not saying your names are wrong since I can’t see all your code and it’s a subjective thing anyways. Just keep in mind you can always shorten names while coding and go back later and make them more clear if it’s needed. Whatever you do always avoid global state that isn’t truly global, and it almost never is. That’s just asking for unintended race conditions for consumers of your library. It would be unfortunate if assign local p =: NewParser in two separate http requests panic with oob because you didn’t want type out a word in your parser ;)</pre>_crackling: <pre>good advice thank you. And i named it tokenIs because, well, <pre><code>tokenIs(tokenTypes ...t.TokenType) bool </code></pre> seeing if tokenIs one of the following haha</pre>nevyn: <pre>Given: <pre><code>if p.tokenIs(t, u, v) { ... } </code></pre> You are reading it like: <pre><code>if car(token) is red, green blue then { ... } </code></pre> ...which seems better than "if is car ...", but IMO what you should have is something more like: <pre><code>if p.isType(t, u, v) { ... } </code></pre> ...and read it as: <pre><code>if the car(p) is the colour(type) red, green or blue then { ... } </code></pre></pre>bonekeeper: <pre>You probably want to keep doing that instead of using package-level variables (globals), if you want your code to be "threadsafe". Imagine two parsers running at the same time in different goroutines depending and altering on a package-level global state. Unless you wrap everything with a lock (ugh!), you'll get undefined behavior. If you do lock, you kill performance and parallelism.</pre>cs-guy: <pre>When the type of value doesn't allow you to make it a const.</pre>