18 ms·
Golang landmines
- tptacek 10y agoBy "never do what GetACat()" does, I think the author means "always return structs, not interfaces".
- barsonme 10y agoI disagree. Return interfaces where it makes sense. E.g., returning type Buffer interface { io.Writer io.ByteWriter WriteString(string) (int, error) ... } from `bytes.NewBuffer' would be a bit absurd. But returning an interface (`io.Reader') from `io.MultiReader' makes more sense than returning some sort of structure. I think the author was getting at how one should always return explicit zero values _or_ explicit non-zero. E.g., func GetACat() (Cat, error) { cat, err := getCatInternalOrSomething() if err != nil { return nil, err } return cat, nil }
- TheDong 10y agoThat doesn't work. Here's the even more subtle version of that bug: https://play.golang.org/p/IVQ9MwyDGP https://play.golang.org/p/IVQ9MwyDGP What you actually need to be careful to do is always return interfaces when possible because mixing interfaces and structs breaks shit. You then also have to always return explicitly 'nil' when you mean nil. That's what GetACat messes up; if you want to return nil from a function that returns an interface, never return a struct you know to be nil, always return the literal value nil.
- tptacek 10y agoThanks, both of you!
- bluejekyll 10y agoCan someone explain what a nil interface is and why it's useful?
- TheDong 10y agoI'll give it a shot. I'll assume you know what an interface and struct are in go. Conceptually, an interface in Go can be thought of as two things: a concrete type, and a pointer to the implementing struct of that type. For example, if you have `var err error = &os.PathError{}`, then the `err` variable is of type `error interface`. That `error interface` simply stores `{type = os.pathError, value = &os.PathError{}}`. The need for the interface to have the value and the type is obvious if you consider that interfaces may have methods called upon them (so they must have an implementation), but may also be used in type assertions / switches (so they must know their underlying type). Go, however, insists that each type has a zero value so that it may be instantiated simply, and interfaces are no different. A user may type `var err error` and create a "zero-value" interface. What should that logically be? Well, it absolutely has no way to have an underlying concrete struct since go does not provide a way to specify a default implementation for an interface, and the type is unknown too.. so it makes sense that it takes the form `{type = nil, value = nil}`. That's a nil interface. A nil interface is useful in many cases. One common one is to indicate the lack of a return value, such as an error. `if err := f(); err != nil` is a common pattern, and takes advantage of the existence of a nil interface, since the function must have returned one (via something like `func f() error { return nil }`). It's also useful to have a nil interface when instantiating something sometimes. For example: var x myInterface // x must be *something* here if condition1 { x = func1() } else { x = func2() } It's mostly useful for consistency with other parts of the language in that things have zero values. I think the only alternative would be for an interface specification to include a "default implementation" to be used as its zero value, but that would not work if a package defines an interface of what it takes in (it would just define a default zero of a sentinel 'zero' value to error on), nor for the `interface{}` shenanigans. Hopefully that helped, but if it didn't please ask a more specific question.
- skybrian 10y agoI'd say the root cause is insisting that each type have a zero value. Logically, this makes no sense. Types are basically sets. It makes perfect sense to define the set {1,2,3}, without any zero value. However, if we think of a variable as a memory location, it's very convenient to be able to declare variables without explicitly initializing them.
- chrisdinn 10y agoWhy would you return an interface type? My advice is don't. Interfaces should be tightly scoped and defined by the consumer not provided by your package.
- Matthias247 10y agoI think it's the other way around. In your package you define an interface and a struct/implementation. The contract to the user is the interface, the struct is merely an implementation detail and ideally shouldn't be public anyway so that are are free to change it. Then you provide functions like NewCat(...) which provide instances of the interface, and which always returns either a valid implementation (non-nil type,non-nil impl tuple) or nil. The user can then directly use this like var cat Cat = NewTabby(). If you instead would return a struct pointer and the user does var cat Cat = NewTabby() you would actually triggered exactly this error. Instead he would need to check what the function returns before assigning it to an interface.
- chrisdinn 10y agoThe issue you describe is not something you should design around. I have written/reviewed hundreds of thousands of lines of Go on projects large and small and have never encountered this problem. However, in practice there are very good reasons not to use interfaces as you suggest. If you make an interface the contract with the user you will find it is now very hard to change. Everyone who had once implemented it no longer does. If I return a struct and consumers use tightly-focused interfaces for only the methods they depend on, I can continue to add methods or modify the struct as I see fit. Think of all the engineers who will create special versions of your interface for testing. You add a new method and every single one of those needs to be updated, even if the overwhelming majority have no interest at all in the method you've added.
- sheeshkebab 10y agoI've run into for loop go routine one just the other day... reminded me of groovy and it's closure scopes too.
- 35bge57dtjku 10y agoIsn't that the same shitty problem Javascript has?
- zzzcpan 10y agoFor me channels and append() are the biggest language level landmines. Not so big are nils, :=, lack of panic when you go beyond last element of the slice, i.e. ([]int{0,1})[2:]. Libraries are a lot worse though, they are minefields.
- sigill 10y agoDo you propose having another syntax for getting the zero-length slice just past the last element? The [2:] syntax giving you {} is a pretty natural progression from [0:] giving you {0, 1} and [1:] giving you {1} in your example.
- retroencabulato 10y ago> Everyone expects these values to be scoped inside the loop, but go reuses the same memory location for every iteration. I don't see this as surprising as a C++ user. Who is 'everyone' here?
- Vendan 10y agoPeople coming from highly dynamic languages, like python and such, most likely
- enneff 10y agoHow would this make any sense, though? In the case of "range" I can see the confusion, but when you explicitly declare a variable and increment it for each iteration of the loop, the only way it can work is if there's one instance of the variable.
- zamalek 10y agoIt's the disparity between how a human thinks and how a machine thinks. Systems languages tend to care less about humans think, which I prefer because humans don't have consistently defined logic.
- enneff 10y agoI don't think so. As programmers, we have to think logically, and in this situation there really is no other logical way of thinking about how that code might work.
- pjmlp 10y agoOnly C and its direct descendants. There are saner alternatives, more human friendly, to systems programming.
- teraflop 10y agoPython's behavior is the same as Go's. For example, this code: l = [] for i in xrange(3): l.append(lambda: i) print [f() for f in l] prints [2, 2, 2].
- karma_vaccum123 10y agoIf only one could go back in time and fix the weird overloading of nil for pointers and interfaces...it is the one breaking change to the Go1 language guarantee that seems worth it.
- kenshaw 10y agoCould you explain what you mean? I don't think many experienced Go programmers would agree with you, so I'm curious as to your rationale.
- dilap 10y agoI don't think it's worth breaking compat, but I agree it would've been nice and saved a ton of confusion. Right now a pointer to a type that is zero is called "nil". Similarly, an interface that contains zero for both the type and the value is called "nil". This is really confusing because var foo SomeType = nil var bar SomeInterface = foo fmt.Println(bar == nil) // prints false, which is confusing If instead you called the zero-value of interface something else, say "unset", it would be a lot less confusing. var foo SomeType = nil var bar SomeInterface = foo fmt.Println(bar == unset) // prints false, which makes sense, because bar is set -- to (nil, SomeType) I first encountered this idea in this reddit thread: https://www.reddit.com/r/golang/comments/4injtt/understanding_gos_nil_value/d2zkmyu https://www.reddit.com/r/golang/comments/4injtt/understandin...
- kjksf 10y agoIt's hardly a better solution. In Go nil is a special identifier that represents a "zero value" for several types (pointers, interfaces, functions, slices, channels). "unset" would be a "zero value" for interfaces so that nil could be "interface that has a type but whose value is an zero value of a pointer or function or slice or a channel. This is inconsistent - now you have 2 identifiers to represent a concept of "zero value".
- Someone 10y ago
- dpc_pw 10y agoGolang is truely modern C. Popular, "simple" (in the twisted sense) and riddled with landmines. :)
- kjksf 10y agoTell me the language that you're using and I'll find multitude of warts and "landmines". 2 out of 3 listed in the article (scoping of loop variables and variable shadowing) is the same in pretty much every other language, for good reasons. The semantics of nil interface are surprising on first encounter, but when you think about other possible semantics, it turns out it's the best solution. Or at least I haven't seen a proposal for how it could behave better.
- pcwalton 10y ago> Or at least I haven't seen a proposal for how it could behave better. Java and C# just have null interface values compare equal to null. I've never seen this cause problems and have never heard any complaints about this.
- _ph_ 10y agoBut you can not call methods on null interface values. In Go that is perfectly reasonable and safe to do.
- deleted 10y ago[deleted]
- pcwalton 10y agoThat doesn't matter. Calling methods and performing comparisons are two different things. I covered this here: https://news.ycombinator.com/item?id=12522941 https://news.ycombinator.com/item?id=12522941
- eternalban 10y agoFully agree and not much of a surprise given that the intersect of the set of principles involved is non empty. https://www.dreamsongs.com/RiseOfWorseIsBetter.html https://www.dreamsongs.com/RiseOfWorseIsBetter.html Note that Go was invented to address a very specific socio-economic issue that Google was facing. Go's simplicity is syntactic. The complexity is in semantics and runtime behavior. It is an economic trade off. A language with a 'complex' syntax, such as Rust, is intimidating to the novice and presents a problem of how to 'integrate' a newbie with an evolving codebase written by old hands. It is true, imo, that ultimtely, the initial cost haircut exacted by languages such as Rust/Scala/Haskell pay substantial dividends, but there is the 'socio-' aspect of the context for you!
- perfmode 10y agoIt should be stated that entire teams build large systems with Go on a daily basis and don't step on these. The community of professional Go programmers has arrived at practices that avoid such issues. However, I wonder how many new programmers fall off early because of these corner cases.
- MaulingMonkey 10y ago> It should be stated that entire teams build large systems with Go on a daily basis and don't step on these I strongly suspect they do step on them, and then simply learn the behavior at that point, and then get on with their lives. > The community of professional Go programmers has arrived at practices that avoid such issues. I'd be interested in knowing those practices. Shadowing I'd assume is "solved" with warnings (as errors)? I don't know enough Go to know how you'd effectively avoid the non-nil interface to nil instance issue, short of carefully checking for nil on every conversion. As for the for loop scopes, do you just declare new variables in the local scopes? Interestingly, I expected the "WRONG" behavior in 2 of the 3 for loop examples... and C# actually changed their behavior (in C#5?) from "WRONG" to "OK" with for-each loops.
- jzelinskie 10y ago>Shadowing I'd assume is "solved" with warnings (as errors)? Yep, shadowing variables are caught by the linters built into every golang package for major editors. Using these linters is built into the culture and as a result they tend to crop up less than you think for such a glaringly obvious issue. >I don't know enough Go to know how you'd effectively avoid the non-nil interface to nil instance issue, short of carefully checking for nil on every conversion. You let it panic. This is a programming error, which is what panics are for. If you caught the error, the proper thing to do would be call panic manually. >As for the for loop scopes, do you just declare new variables in the local scopes? Interestingly, I expected the "WRONG" behavior in 2 of the 3 for loop examples... and C# actually changed their behavior (in C#5?) from "WRONG" to "OK" with for-each loops. This is subjective, and easily testable. Most early Go developers learn to test their understanding of "for loop scoping" after they implement an infinite loop that expects `defer f.Close()` to close the resource, but instead it builds up until the kernel kills the process for not releasing file descriptors. --- I was recently surprised to find out that `type MyError error` and switching on types like `switch variable.(type)` can be a bit scary [0]. Basically you should always define your new error type from a string and never attempt to re-use the builtin error type. In retrospect it makes sense, but you'd naively think this is the proper way to create a new error type. [0]: https://play.golang.org/p/3BSKu8Cd4o https://play.golang.org/p/3BSKu8Cd4o
- Animats 10y agoLanguages probably shouldn't allow shadowing, certainly not within a single module. Think of the maintenance programmers. "Defer" was probably a bad idea. It's a run-time construct - you're adding to a to-do list run at the end of the function. It's not scope oriented. Something like Python's "with" clause would have been both simpler and more useful. "with" calls "_enter" on an object when you enter a scope, and "_exit" when you leave, no matter how you leave. All the file-like and lock-like objects support "with". Python's "with" even supports exceptions properly - if you're handling an exception when "_exit" is called, the "_exit" function gets told about the exception, so it can clean up and then re-raise the exception. If an "_exit" itself needs to raise an exception, that works, too. "Defer" is blind. With "defer", nobody ever finds out that the close failed to write the end of the file.
- smarterclayton 10y agoTo be fair I think most Go programmers are exposed to defer .Close() on files via example, and it's possible every copied example can be traced back to some proto-example in the very early days of Go. It is extremely pernicious in the ecosystem - Docker, Kubernetes, and etcd all had rounds of cleaning it up over the last year or two. I would expect linters to flag Close without checking the error value (if they don't already).
- Animats 10y agoWhat can you do if you detect an error in a "defer" clause? Besides panic?
- ansible 10y agoWell, I don't know if panic is the right option. However, checking the return value is always a good idea, and any errors should at least be logged. I too wish it was less cumbersome to use defer properly.
- giovannibajo1 10y agoOne thing you can do is simply use BOTH a defer Close and a straight-path Close with error-checking.
- oppositelock 10y agoI've been doing quite a bit of Go programming for a while now, and I've internalized all these behaviors, but this one gets me every time: var err error if true { foo, err := someFunction() // err gets shadowed } It's particularly annoying because if both foo and err are defined in the outer scope, you get a compiler error that no new variables are on the left, so you tend to forget that removing one shadows the other.
- lopatin 10y agoI tend to simply not have duplicate variable names (with the exception of `err`, which is always treated with special care anyways) so this shadowing behavior doesn't really trip me up any more.
- kyrra 10y agoNot sure it would help you but adding "go vet -shadow" to some kind of test or build script may help you detect these problems. https://golang.org/cmd/vet/ https://golang.org/cmd/vet/
- leetrout 10y agoThis. If anyone is using VIM or Atom please please please install the Go helpers, too. Atom's go-plus is awesome. The team I work with- I beg them to use linters and go-plus and avoid these pitfalls like shadowing err. Instead most of them will run the linter, not fix the errors, then defend poor practices like shadowing and returning private types from exported methods.
- omginternets 10y agoI think Go is one of those languages that really benefits from an editor that performs static analysis. I use Atom and go-metalinter catches this on a daily (if not hourly) basis. Variable shadowing is a sharp edge of the language that's very effectively dulled by proper tooling.
- paulsutter 10y agoDeclaration assignments should be more explicit. For example: foo, err ::= someFunction() // explicitly declare both foo, err := someFunction() // declare only foo, err must exist Then you get all the convenience without surprises.
- blt 10y agoSeems that the explicit lambda capture in C++ is a good way to avoid the first issue. Implicit capture of closure variables is kinda scary when you think about it.
- mmargerum 10y agoI don't know if I'd call this a land mine, but I wish it didn't work because it confuses beginners. You can type for i:= range myvalues In this case i is the index of the value. I'd rather they didn't allow this and instead you would just type for i,_ := range myvalues
- oconnor663 10y agoI would add taking a value receiver when you meant to take a pointer receiver.
- kowdermeister 10y agoMaybe it's just me, but in these cases when I encounter some weird behavior in a language I assume that I don't know enough of the language. It's actually not the designers fault that they are there, but my knowledge that requires more info.
- coldcode 10y agoAs a Swift programmer Go programming seems riddled with a need to be perfect. I'd rather the language help me not make errors rather than assume I am perfectly aware of all the gotchas. Swift isn't perfect but it does make things less likely to confound you later.