top | item 19218097

Practical Go: Real-world advice for writing maintainable Go programs

688 points| ra7 | 7 years ago |dave.cheney.net | reply

226 comments

order
[+] briatx|7 years ago|reply
> Naming the Config parameter config is redundant. We know its a Config, it says so right there.

> In this case consider conf or maybe c will do if the lifetime of the variable is short enough.

This seems petty. Is it really that problematic to type out a few extra characters?

[+] pcwalton|7 years ago|reply
Yeah, I used to go by this advice, and I found the maintainability of my code dramatically increased when I typed out full names. I don't even use "i" for loop variables anymore. If the length is a problem, invest in an editor with autocomplete.

Well-known abbreviations are fine, like "iter" and "prev", but single-letter variable names notoriously impede readability for me.

[+] networkimprov|7 years ago|reply
And this is worse advice:

> Functions should do one thing only. ... In addition to be easier to comprehend, smaller functions are easier to test in isolation, and now you’ve isolated the orthogonal code into its own function, its name may be all the documentation required.

Using single-caller functions as a substitute for comments makes the workings of a specific operation much harder to follow, as you have to jump around the source to understand its effects.

A long function is easier to understand than an exploded one.

Also tests should target specific operations (aka functional tests), not every single function in the program.

EDIT: Every function you add becomes part of your internal API. Any API, exported or not, should comprise a cohesive collection.

[+] sythe2o0|7 years ago|reply
The argument is that it is not fundamentally better to use the longer name in the given context, so why make it longer? I'd say it isn't about how long the variable takes to type either, but how long the code takes to read and parse.
[+] tdb7893|7 years ago|reply
I'm wrestling with this at work right now and the short names don't really bother me as they are right in that the fact that if you have the type the shorter name can make the code easier to read slightly. What annoyed me is that for those single letter names I always got collisions so my naming was inconsistent. I personally ended up doing medium length names (like conf) except for the case where there was only 1 local variable (and sometimes if there were two or three) because in that case there were no collisions and what that variable was is very very clear
[+] sacado2|7 years ago|reply
It is problematic for readability reasons as long as you are making complex expressions:

    configuration := NewConfiguration()
    configuration[word] = parameters[values[line][column]] - parameters[values[column][line]]
vs

    conf := NewConfig()
    conf[w] = params[vals[i][j]] - params[vals[j][i]]
It takes your brain more time to parse the first line. Now, there is an obvious limit, this is probably too much

    c[w] = p[v[i][j]] - p[v[j][i]]
unless maybe the scope of the vars is very limited.
[+] ilovecaching|7 years ago|reply
It says so in the signature, but Go code tends to have long functions, so it's kind of a fallacy to say that it's going to be easily recognizable 50 lines in because it's in the signature. Now if this were Haskell and it was a single small expression, it would be different.
[+] akerl_|7 years ago|reply
The issue w/ calling it "config" is that you end up with the confusing scenario where "config" is the object and "Config" is the type, differing only in casing.
[+] zerr|7 years ago|reply
They don't use any IDEs or modern editors, but vim as a bare bones text entry, without syntax highlighting or word completion.
[+] karmakaze|7 years ago|reply
Also in this case Go makes you use types in the name since it doesn't allow function overloading.
[+] letientai299|7 years ago|reply
Came from Java world, I find the Go comment and godoc are really limited. We can't link between functions, types. We don't have a standard way to declare input, output, don't have any distinction between a normal word and a Go identifier Refactoring using tool (e.g: Goland) usually lead to unexpected text replacements.

Take following functions, for example:

  // Auth check if user credential is existed in bla, bla...
  // ... (many more explanation)
  Auth(user User, authProvider AuthProvider) bool


  // AuthDefault check user against the default **AuthProvider**
  AuthDefault(user User) bool {
    return Auth(user, new(DefaultAuthProvider))
  }

If only the AuthProvider in the second function's godoc be a link to the first one, we don't have to repeat the explanation for the second one. Dev will be able to discover the explanation easily via their IDE. This alone will be very helpful for the maintainability of any big projects.
[+] dilap|7 years ago|reply
Flip-side is less complexity and the doc comments in raw code look better. Go's generally all about, "how much can we strip away and still have things work".

Your criticisms re refactorings and links are on point though.

Trade-offs.

[+] adamlett|7 years ago|reply
The problem with the Java way, as I see it, is that it puts more of a burden on the programmer to the point where a majority simply won't do it. Getting programmers to document their code is an uphill battle to begin with. But the bureaucracy that Java(doc) imposes makes it even harder to win in all but the most elite institutions. What I see a lot, is IDE-generated documentation which can be automatically inferred from the code, and is therefore redundant. While it superficially looks like documentation, it really is just line noise. I would agree that Java documentation at its best can be better than Go documentation. But I suspect that the average Go codebase is better documented than the average Java codebase.
[+] TheJazi13|7 years ago|reply
I’d argue there’s a failure of the code to some extent to need such extsenibility with documentation. I’ve often found when I need more info outside of Godocs that simply clicking through and diving into the source provided me with all the answers I needed to know.
[+] jerf|7 years ago|reply
The authors have chosen to make it as limited as they have on purpose, as I understand it.

I disagree with them, but here we are.

I've been slightly tempted at times to make something with markdown but never gotten enthusiastic enough to put the time. Plus, markdown is technically a bit too powerful, so I'd want to cut it back down, and before you know it I'm inflicting the 3,124th custom markdown on the world. Really I just would like bold, italic, monospace, identifier linking as you say, and numbered and bulleted lists not in the unformatted text wrapper.

[+] fwip|7 years ago|reply
You should put the docs for AuthProvider on AuthProvider, not on Auth or AuthDefault.
[+] OutsmartDan|7 years ago|reply
Would love to see better separation on the page between "Bad" and "Good" examples. While a good read, it's difficult to quickly distinguish between the two.

For example: https://github.com/airbnb/javascript

[+] andriosr|7 years ago|reply
true. Google will index it and I can see many people copy and paste bad code considering the best practice for doing something
[+] doodpants|7 years ago|reply
I'm not a Go user myself, but I skimmed through the document. What I found was a lot of good programming advice that is generally applicable, rather than being limited to the Go language. (It seems most of the Go-specific stuff is in section 5.) It's always good to see robust coding practices being promoted, regardless of language.
[+] diegs|7 years ago|reply
About half-way through and I think this is a great article, in particular the quotes and I also agree that the first 4 sections are generally applicable.

One thing I disagree with is the remark about having fewer, big packages. Though conceptually I agree that avoiding having too many public APIs that aren't widely used makes sense, in practice--at least on the types of projects I tend to work on--I find that directing people to split things into a few packages forces them to think about a decoupled design with good APIs between the components. This could certainly be done with discipline inside a single package, but unless everyone working on the codebase is very diligent about this it's easy for abstraction leaks to creep in.

Ultimately it's a judgment call, but I think an earlier paragraph (copied below) is far more important than optimizing on having fewer packages or fewer exported types and functions, especially (as is also pointed out in the doc) you can use `internal` subdirectories to make APIs project-private if you are writing a library that is consumed by other projects, as opposed a service.

> A good Go package should strive to have a low degree of source level coupling such that, as the project grows, changes to one package do not cascade across the code-base. These stop-the-world refactorings place a hard limit on the rate of change in a code base and thus the productivity of the members working in that code-base.

[+] jerf|7 years ago|reply
I agree with you. I like the way packages are isolated from each other, meaning that to understand a package it is usually by definition a good start to simply read what is there. Smaller packages mean more bite-sized chunks of the program. And I think the discipline of slicing up your program this way is very, very good for the design, and often makes the tests easier to write to by significantly shrinking the surface of what your tests have to "fake" in order to test your code. I think it's just a whole heapin' helpin' o' benefits.

However, I feel myself to be in the minority on this one. To which I basically shrug and write my code with lots of relatively small packages. It really only affects code you're working on, or that your team is working on. Things you pull in as libraries and have no direct interaction with don't matter too much on this front.

[+] atombender|7 years ago|reply
I often find myself wishing Go either allowed import cycles between packages, or allowed namespaces within a package. Because they can become unwieldy.

For example, a common convention is to avoid redundancy. Let's say you have a package "builder". Your encouraged to have "builder.New()" as a constructor, not "builder.NewBuilder()". Fine. Now let's say you need two types of builders: One for building "schemas", one for "objects". Your original constructor now needs to be something like "builder.NewSchemaBuilder()" ("NewSchema" would be confusing, since the function creates builders, not schemas). Or maybe turn it into a verb: "builder.BuildSchema()", "builder.BuildObject()". Part of this is due to the lack of statics, or we could've had "builder.Schema.Build()" or something.

You can also split the package up into two packages, one for schema building and one for objects. (Naming here can be tricky, too. Will it be "schemabuilder.New()", or "schemas.NewBuilder()"?) But if the two builders need to share types, you may end up refactoring the common types into a common package that exists only because of the split.

I have a concrete example of this right now for a small query language. The language has data types (common interface Type) and values (Value). Values can tell you their type. Types can construct values. But they're two distinct sets of declarations. The type implementations and the value implementations can't easily be split across separate packages without having a common package that exists only to hold the shared types. It'd be nice to have "types.String" (in types/string.go) and "values.String" (values/string.go), not "lang.StringType" (lang/string_type.go) and "lang.String" (lang/string.go).

Having a namespace option would help here. Everything could be in one package, but under separate namespaces.

[+] apoorvgarg|7 years ago|reply
Yes, and I think the quoted paragraph has so much more to do with coding around interfaces (behaviour) than with abstraction using non-exported package symbols.
[+] bsaul|7 years ago|reply
This has got to be one of the best and most useful post i’ve seen about go (and code design in general) in a long time.
[+] arendtio|7 years ago|reply
> A good Go package should strive to have a low degree of source level coupling such that, as the project grows, changes to one package do not cascade across the code-base.

I wonder though why there is so little emphasis on how important interfaces are in Go. I mean, section 4.5 talks about it a bit, but in my experience, this mistake is made far too often.

[+] argd678|7 years ago|reply
> 2.1. Choose identifiers for clarity, not brevity

This is a real problem in Go these days, people use one and two letter vars quite a bit which makes reading code you’re not familiar with practically impossible. On one project we simply switched to semi-java length like names since our customers couldn’t read the code.

[+] entity345|7 years ago|reply
Since when are variables names a language issue?

There is no such thing as "Java length like names", or whatever. There are good practices and bad practices and they are universal.

[+] twentythree|7 years ago|reply
> 2.3. Don’t name your variables for their types

What's the best way to handle a situation where you use two different types for the same data? e.g.

  var usersMap map[string]*User
  var usersList []*User
[+] Groxx|7 years ago|reply
https://dave.cheney.net/practical-go/presentations/qcon-chin...

> 8.3. Never start a goroutine without [knowing] when it will stop.

100% agreed with the concept, but even the final example is flawed. That'll unblock and continue immediately after `close(stop)`, without being able to do two important things: it can't tell you when it's done shutting down, and it can't tell you if it encountered an error. Fixing this makes it even more complex.

Both of those are common and often necessary things to do - e.g. if you need to drain traffic, you need to wait until it's actually done before killing your process. Same goes for closing files (you might need to flush / sync first), OS resources that might survive your process, etc. This example will just kill your process as soon as everyone has been informed that it should stop, not when they're done.

---

Yeah it's a bit nitpicky, but it's even called out as "asking a http.Server to shut down is a little involved, so I’ve spun that logic out into a helper function". Except that the helper function can't ensure the involved server shutdown process actually shut down the server. That's a dangerous pattern to encourage, and it's an over-simplification that's absolutely rampant in go tutorials and code I encounter.

The workgroup lib he links doesn't (arguably?) have this flaw, but it has some strange / incomplete error handling details... E.g. if you Add 5 funcs, you'll only receive the error result of the first func to complete, not the first err or anything more predictable. If the first succeeds but all the rest fail, you'll see no error. Go's errgroup does "first non-nil error" at least, but you can still only get one: https://godoc.org/golang.org/x/sync/errgroup

[+] gigatexal|7 years ago|reply
This is so fortuitous as I started writing my first real Golang service and as a python dev I have no idea what I am doing.

But one refreshing thing is how opinionated the language and the frameworks are refreshing as there’s only one acceptable way to do many things.

[+] hashhar|7 years ago|reply
I agree so much with this. I recently attended a Golang meet up and since I was one of the few who had been using the language regularly for over 2 years now i was asked to let the newcomers know my favorite "feature". I replied with the same point you make above and was met with a look of refreshment on people's faces.

Go lets you write One True Code for the most part.

[+] everybodyknows|7 years ago|reply
I have found this advice spot-on for more subtle questions, over my last two years of learning and building with Go full time:

https://golang.org/doc/effective_go.html

Also see back issues of the Go team blog for insight into concrete implementation realities of slices, interface types, GC and more.

[+] tarasmatsyk|7 years ago|reply
Good collection of best practices overall. I would not say they are Go-only, as I read about them 5 years ago in Clean Code by R. Martin and another part can be found in Code Complete by Steve McConnell.

Once again, good collection if you have no time to read the book. Anyway, it does not cover other parts like contracts/interfaces, the absence of comments (which can be a good sign) etc. Would recommend checking both CC books if you want to write better and maintainable code.

PS. Anyway, Dave, thank you for the popularization of best practices.

[+] leshow|7 years ago|reply
I've always found Go's 'guiding principles' to be highly subjective, the annoying thing about it is that they are masqueraded as objectivity. Simplicity and readability for whom?

If you've been writing C and Java for years, all simple/readable means is 'familiar'. There are other definitions of simplicity

[+] shoo|7 years ago|reply
> That’s a lot of repetitive [error handling] work. But we can make it easier on ourselves by introducing a small wrapper type, errWriter.

> errWriter fulfils the io.Writer contract so it can be used to wrap an existing io.Writer. errWriter passes writes through to its underlying writer until an error is detected. From that point on, it discards any writes and returns the previous error.

this is a cute trick.

In some cases it might produce a great result. For example, floating point nan values work roughly like this: a special error values included in the type that can be computed upon without needing to write special control flow until you explicitly need to check the final computed result to see if you got an actual value or the nan value.

but I fear the applicability of this trick in go is limited as it relies on a heap of idiosyncracies belonging to this example:

The chain of potentially error-ing operations that are being performed all involve a value of the same type that can be wrapped. In a less contrived example there would be a variety of different types involved in the sequence of possibly error-ing operations.

The code that is processing the wrapped error-hiding type roughly must not perform side effects when processing an apparently non-erroring computation, since now the error wrapper type is stopping the code from aborting early. It's completely non obvious that this transformation will be correct (ie result in a program with equivalent behaviour to the original program) without reading exactly through the implementation of everything that touches the error wrapper. This isn't helped by go not having a way to tag functions as pure.

The new code now superficially looks like it is wrong as it isn't bothering to check error values of the functions it is calling in the usual way. This will probably cause a spray of linter warnings about failing to handle possible errors in your CI build.

In other languages a much more general way to address this kind of problem could be:

error monads ; or exception handling

Which can be used to short circuit the entire normal control flow.

[+] masklinn|7 years ago|reply
> In other languages a much more general way to address this kind of problem could be: error monads ; or exception handling

errWriter is quite literally "let's do monadic error handling, except not just without monads but without generics or reified errors". And thus it can only handle a single source or error type.

Also FWIW monadic error handling != error monads. For instance Rust does the former, but its type system can't express the latter (so there is no monad or functor abstraction on top of Result).

[+] johnisgood|7 years ago|reply
I don't understand something.

> 5.1. Consider fewer, larger packages

> 5.2. Keep package main small as small as possible

If you have one package, which is the main package, then how are you supposed to keep it small, and at the same time not creating more packages because somehow that is the work of the devil? He is telling me to try to fit everything into one package, but at the same time keep it really small. It is not going to work. Maybe I misunderstood?

[+] dub|7 years ago|reply
In golang packages named "main" are special. They're not importable and they correspond to a single executable.

If you want to be able to reuse code in different packages in the future or have more than one compiled binary, you won't be able to put everything into a single "main" package

[+] hactually|7 years ago|reply
Your main package can sit under `cmd/foo/*.go` and import a few larger packages

That's what it means.

[+] jstewartmobile|7 years ago|reply
Love Mr. Cheney, hate stuff like this.

Someone who has put the hours in will probably stray from all of this advice, and still end up with something beautiful. Meanwhile, for all of the other schmoes who haven't put the hours in, this is just more fuel for screeching about how this name isn't right or that comment is too long.

Dijkstra said GOTO was bad, and now we have callback hell and 10-layer inheritance hierarchies instead.

[+] AnimalMuppet|7 years ago|reply
If you're writing the kind of thing where you need callbacks, GOTO isn't going to make it more clear what's going on...
[+] dana321|7 years ago|reply
My advice would be this: If you have a specific reusable component.. Make a package to contain it.

Use init() to compile any regular expressions and store them as variables within that package, so that you don't

Split out related code in a folder, remembering that the included code is done in alphabetical order - so shared functions within the same package you should alphabetically make it the first to be included.

You can register components of a larger system by using the init() function to call a function in the base package, and within the calling package (usually main) import the "plugin" using the underscore space prefix (sql database drivers use this method)

Use the new go modules system; its great. But sometimes it doesn't include the latest packages, just modify the go.mod file and anything after the package name remove it and just put master instead.

One last thing with error handling. Everybody does if err!=nil. I tend to go the opposite way, if err==nil and nest these, if err isn't nil i drop out and deal with the error. If it is ok, it will return ok.

[+] undreren|7 years ago|reply
> One last thing with error handling. Everybody does if err!=nil. I tend to go the opposite way, if err==nil and nest these, if err isn't nil i drop out and deal with the error. If it is ok, it will return ok.

Don't you get "staircase of doom"?

    err := unsafe1()
    if err == nil {
        err = unsafe2()
        if err == nil {
            err = unsafe3()
            if err == nil {
                err = unsafe4()
                    if err == nil {
                    err = unsafe5()
                    if err == nil {
                        // ...
                    }
                }
            }
        }
    }
[+] jimsmart|7 years ago|reply
> Use init() to compile any regular expressions and store them as variables within that package

No need to resort to init() for regexps, simply use an unexported package level variable:

var validFoo = regexp.MustCompile(...)