Uber Go Style Guide
github.com
github.com
This could be used as an ad for Rust borrow checker, verbatim. You can't modify a map or slice you passed as an argument if a reference to it is stored!
For the record, rust does also default to immutability, and you can share immutable references all you'd like.
Ownership and move semantics are checked at compile time, and allow effective "not sharing" without waiting or defensive copies.
More typically you're building a new slice containing the results of a query or other computation, and returning it without keeping a reference. Or instead of exposing an internal map, you have a Get method.
It comes up in any language that allows shared mutable state, so not much in functional languages which discourage mutability or Rust which discourages shared mutability.
On the other hand, most code tries to avoid shared mutable state by convention, there’s a nice bit in the Go lang design article that basically says convention is good enough, significantly simplifies the language, at the cost of the odd bug.
Many Go libraries like to offer passing a byte slice to reuse as a destination. Many Go libraries take byte slices or structs containing them as arguments. A common result is "loop over some input, read it into the reusable slice, pass the slice to the next step". It even sort of works - until the next step does something asynchronous.
It doesn't help that:
- Lots of things return a new slice "sometimes" - _this_ append() result is safe to pass on; this other one is not; this other one "it depends".
- The `x[:]` idiom to turn an array into a slice looks exactly like a Python copy, but actually shares the same backing store.
In the end we basically banned reusing slices as a result. (The rule I tell new programmers is "if you pass the slice elsewhere in a loop, the slice needs to be allocated within the loop.") Which is a shame, because the performance gain from safe reuse is often significant. But until the type system blocks us from changes, usually in the oblivious callee, which turn a safe use into an unsafe one, it's simply too much effort to remember and review each case.
var buf bytes.Buffer
buf.WriteString("foo")
b := buf.Bytes()
fmt.Printf("b == %v\n", string(b)) // b == foo
buf.Reset()
buf.WriteString("bar")
fmt.Printf("b == %v\n", string(b)) // b == bar
In reality it's part of a different language which exposes a lot of complexity to the programmer as opposed to Go, which religiously avoids that.
I think that rust fills the needs of applications with extreme requirements. A browser is in that niche. Not all software is.
Thus most of the time you don't have to inflict yourself the pain to work with it.
Anyway... Languages with GC still do have ownership. It is just defaulting to be the most problematic case of "shared ownership" everywhere. Shared ownership is only worry-free if you don't ever modify data. Which is possible, but actually harder and more constraining to implement in a practical software (especially if you care about any performance). So then you either have to synchronize everything with mutexes, and figure out a way to not deadlock yourself on every step (also hard), or you have to implicitly track ownership and decide who can and when mutate the data, which is exactly what Rust would make you do, but this time you get no compiler help. The only reason people don't notice that is because they do it incorrectly and it works most of the time, so they are unaware.
There is no escape from it - one has to figure out ownership with a GC or without it, because GC has nothing to do with ownership. It only reclaims memory after all references are gone. That's it. That is actually very, very little improvement.
Ironically, that's Rust biggest problem: not using perfect ownership management works for most software most of the time and that's good enough for most people and companies.
You probably can't sell ownership management as a feature. The only thing that Rust can sell and that makes sense is "it won't crash" + "it's fast", but then there's a lot of other languages that have the first property and the second is not very appreciated, as seen in the rise of so-called Electron apps and web apps in general.
I think in time more and more people and teams will discover that doing stuff with Rust is just cheaper and more productive. You learn ownership once, you get great results ever after.
It seems likely that you and I are using different definitions, and yours Rust-specific (and therefore not meaningfully generalizable to other languages, especially those with GC).
I mostly write Java for a living, have done for a decade, and still occasionally trip myself up by mutating an object I'd forgotten I've shared.
Go is a very effective tool in the hands of experienced programmers, without having to pay the cognitive load price of 'Mommy' languages. I am reminded of Bryan Cantrill's rant regarding threads in this context.
Writing solid software is hard. It takes skill, experience, and serious battle scars. Tools and languages can help, but at the end of the day, it comes down to the team that writes the code.
This should not be news to this audience on HN: proper software architecture, design, and development processes trump "language" any day, any time, any planet. Conversely, even the most 'profoundly correct' language in the hands of unprepared development teams can result in software disasters.
The human factor is still paramount in software development. That is my experience after decades in this profession and is not mere conjecture.
You can't have the best of both worlds. You win some and lose some. Rust avoids this but loses flexibility.
In Rust, the borrow checker is too a separate static analysis stage and not directly tied to code gen. There are Rust compilers without the borrow checker.
The go authors wouldn't accept it, the ecosystem wouldn't work with it, and it would be fighting an uphill battle rather than just using rust or building a new language.
> There are Rust compilers without the borrow checker
Then those compilers do not compile rust. They compile a bastardized version of rust whereby programs that are not valid rust programs can compile.
Also, can you name these non-borrowck-ing rust compilers? You say there's multiple, so you should be able to at least name one or two.
I didn't realize how horizontal vs. vertical code comparison affects readability; IMO horizontal is MUCH more readable.
Example: https://github.com/uber-go/guide/blob/master/style.md#defer-...
Although on the phone I initially didn't realize there was horizontal content/scrolling! Thanks for pointing that out! Very neat.
It does a few things.
- Most declarations, including method declarations, take place at the top level. This is unlike most languages which declare methods one level indented along with the object's fields.
- Short type definitions (`type T1 T2`) are encouraged and often used for any repeated non-scalar type. This shortens argument specifications and (if a good name was chosen) improves readability.
- Anonymous struct fields are shorter both to declare and use. In class-based OO this is often the case for subclasses but not wrappers/facades. Go's approach covers both.
- Go linters push people to avoid `if x { ... return a } else { ... return b }` in favor of `if x { ... return a } ... return b`. I actually hate this, but it does keep indentation down.
- The lack of exception handling means there is a constant "pressure" to push errors back up manually, via early return if it's locally unhandled or to the top of the current lexical scope of it is. The result is definitely verbose and one of the easiest parts of Go to criticize - but the same pressure results in short (but plentiful) error handling lines.
(I do agree the majority of savings comes from short variable names, though I disagree that it affects readability much for the case of _variable names_ - struct fields are another issue...)
You can do the same in C++ or Kotlin, etc., but you don't find people saying that those automatically make lines shorter.
> - Short type definitions (`type T1 T2`) are encouraged and often used for any repeated non-scalar type. This shortens argument specifications and (if a good name was chosen) improves readability.
Also possible in C++ or Kotlin, among others.
> - Anonymous struct fields are shorter both to declare and use. In class-based OO this is often the case for subclasses but not wrappers/facades. Go's approach covers both.
This can save lines of code (at the expense of not being able to easily figure out what classes implement or embed other classes or interfaces), but does nothing to shorten lines of code.
Same applies to error handling. Returning early can be useful in certain situations, but cause readability issues in others. golang linters did a bad job here as you point out.
I don't know what happens in the Kotlin community, but you absolutely find people saying C++ is "less nested" than Java/Python/JavaScript/etc (and claiming it as an advantage and disadvantage depending on their viewpoint).
I don't know what you're really arguing with here. Go gives you a construct that makes lines shorter than most other OO languages. It's not "automatic" as it's a feature of the language someone had to design - but regardless it is shorter.
> [Short type definitions are] possible in C++ or Kotlin, among others.
Again I don't know Kotlin, but no, it's not possible in C++ (at least up through the 11-and-a-bit I work with). The equivalent C++ would be "struct T1 : public T2", plus another line of "{}". And it's rare (for good-ish reasons) to subclass specializations of the default containers rather than wrap them.
> [An anonymous structure] does nothing to shorten lines of code.
I don't see how this is a debatable point. Not having to specify the field name in the definition is a shorter line than than having to specify it, strictly so. Not having to specify the full path to such an embedded object to call a method or access a field is nearly always shorter (it can be longer if your struct name is longer than your field name would have been and you have an ambiguous resolution - the former happens a lot, the second less, and both in combination even less).
> Returning early can be useful in certain situations, but cause readability issues in others.
Regardless, it keeps the lines short.
You seem to want to argue "Go is ugly", which, fine. But the claim was "Go doesn't do anything to help make shorter lines." It does quite a bit.
Just one comment though:
> Embedding sync.Mutex
Never do this on an exported type. If you embed sync.Mutex that makes "Lock" and "Unlock" part of your exported interface. Now the caller of your API doesn't know if they are supposed to call Lock/Unlock, or if this is a pure internal implementation detail.
your `type Stats` "isn't" a mutex. It has a mutex.
Isn't that exactly what they say?
> Embed for private types or types that need to implement the Mutex interface.
Personally, I would never embed a sync.Mutex though because even for an internal type it is confusing.
HashMap is probably a good example
A lot of people will do things like:
func (s *Server) HandleFoo(ctx context.Context, in *input) status {
ch <- workFor(in)
return status.Accepted
}
func (s *Server) HandleFoo(ctx context.Context, in *input) status {
select {
case ch <- workFor(in):
return status.Accepted
case <- ctx.Done():
return status.Error("handle foo: wait for work to be accepted: %w", ctx.Err())
}
}
My impression from digging around the open source world is that nobody except me cares about this. So maybe I'm just crazy. But I really like not leaking goroutines on long-running programs, making Control-C behave gracefully, etc.
so in your example it should be done like this: ch <- workFor(ctx, in)
and the 'workFor' function should be the one that gets canceled with the context.
Deep down at the very end, you would have select statement with '<-ctx.Done()' and something else that doesn't need canceling (if it needs canceling it should have taken context too).
Of course the world isn't perfect and not everything takes context right now (even in standard library), so you do have to do some workarounds every once in a while.
Where I was going with all of this was that the linked style guide mentions cases where people create buffered channels presumably because their channel writes start blocking. No buffer will make up for the case where the rate of work requested is higher than the rate at which work can be completed. What people really want in that case is the ability to get out of the channel write and return an error to the downstream system; you don't want to buffer, you want to cancel. There are many ways to accomplish the act of cancelling work, but you do have to watch your channel writes explicitly.
select {
default:
case <-ctx.Done():
return ctx.Err()
}
(If you are curious about how many extra time is wasted, it's easy to write a benchmark test for that. Last time I checked it was ~20ns when you are going the default/not-cancelled route)
if ctx.Err() != nil {
//then the context has expired or been cancelled
}Most people will not see the need for cancellation - until they either need to implement a deadline timeout later on, or run into the problem that lots of outdated tasks take up their memory. Those things can come up years after the inital program had been written.
It's also not purely a Go thing. E.g. in Java thread interruptions are also barely handled correct. Or CancellationTokens in C#.
1500?! I can't even imagine how micro a microservice could be such that Uber could have so many.
edit: Having said that, the Monzo blog post also on the front page says it has 1100 microservices, so I suppose I've just not worked somewhere with 'actual' microservices. What constitutes a 'service', a single procedure for RPC?
In general it is easy to underestimate the complexity of systems you are not familiar with. The devil is in the details and deeper you look, more complicated it usually gets.
Very easy.
A key-value store is theoretically easy, “just store data and give it back”, but a distributed / highly reliable key value store is a crazy beast.
Collision detection and response seems like a “solved” problem but perk at the code and you’ll find just how many lines of code a basic game spends preventing players from waking through walls.
- build a UNIX philosophy, "efficient atomicity" so 1 microservice = 1 command
- you then build logic with these like Lego bricks, that's your glue code => backend ties that together
On a typical base Linux install you have something between 250 and 1500 packages. Average Java repo has like, what, 80 files maybe? 120? Break it down to functions / structs and you can see the 1500 being a reasonable estimate of a modular architecture.
So you have one microservice to return a catalog, another to take a search input, another to serialize this stuff, another to autofill something, another to autofill some other thing, etc. Your app architecture schema is produced by/in microservices.
That's the extreme approach to microservice architecture (the 1 command / 1 object / 1 function / 1 struct atomicity Lego-style), and in such a case most microservices would typically look very much alike, with exactly the same "general" parts (auth, db connect, etc) and just your business logic varying in between.
With so small microservices, a team of 4 may be responsible for a good dozen microservices, a team of 10 may handle a hundred... You quickly rise to high hundreds in such scenarios.
Disclaimer: This is extensively parroted from the latest episode of "Go Time", a fantastic podcast imho.
These services include all sorts of worker processes, things that manage queues or monitor things, probers, administration, public interface and configuration. It works really well for us.
Edit: eh, looks like I was not the only one who immediately thought of that :)
Looking at the stock, they probably should focus on building new things/better the business model, rather then rewriting things in Go.
Uber have thousands of engineers to develop and maintain a taxi hailing app...
> As our business grew, so did our resource utilization and latencies; to keep Schemaless performant, we needed a solution that would execute well at scale.
That's the third sentence. Clearly it was not an "emotional decision" and it was in support of their business model (scale).
I still use them quite a bit. Particularly synchronizing large rule sets as bit arrays. Of course you can use sync.Wait instead. But make(chan bool, N) semantics are just more convenient. It's stealth synchronization as a by-product. And hence the warnings about determinism!
The former is a reasonable concurrency primitive. The latter superficially seems similar, but is actually a much more complicated primitive, with the corresponding code understanding problems and the increased likelihood of more concurrency problems being hidden at low scale but coming out at scale (mostly deadlocks), plus the fact it seems similar is also problematic. The fact that the Go type system does not allow distinguishing between the two is also problematic.
It has some special-case purposes, but I also always scrutinize any channel created with buffering to make sure it is one of those special cases.
Buffered channels have not impressed me with their ability to do any sort of performance improvements. In almost all cases, if you've filled up your readers, you really want your writer to block. It provides cheap, effective backpressure; arguably for software-at-scale it's the most useful aspect of the channel primitive.
The other reason to use them is controlling CPU. Spin up a pool of consumers and control how many cores they use with the buffer.
The suggested good clone is not very efficient: https://github.com/go101/go101/wiki/How-to-efficiently-clone...
In fact, I prefer the suggested bad one instead. We should let the caller to determine whether or not a clone is needed.
--------------------------
Start Enums at One: https://github.com/uber-go/guide/blob/master/style.md#start-... Any reason here? (Edit: I just missed the reason. However, I think there should be a default value for most enums and most of the default values should zero.)
--------------------------
Local Variable Declarations: https://github.com/uber-go/guide/blob/master/style.md#local-...
Personally, I prefer "var x = ..." and think short variables should be used as limited as possible. I remember that Go team has a not-very-concrete plan to depreciate short variable declarations.
--------------------------
Avoid Naked Parameters: https://github.com/uber-go/guide/blob/master/style.md#avoid-...
This suggestion has its own drawback. When a parameter name changed, all the places using it must be modified to keep consistent. This is one reason why Go team rejected named arguments.
If this must be done, I recommend to define and use some named bool constants instead.
The rationale is, it’s often quite useful to know that the originator hasn’t specified a value, and to help new originators avoid unintended side-effects.
This style guide has the same effect, but it’s implicit; takes a few more mental cycles to parse in exchange for less code. Personally, I think it’s probably worth the trade off to type it out once and save time thinking later.
If you use closures, it's nearly impossible to compare the results for equality in tests or code that might care to validate / deduplicate / whatever those args. You can achieve it with a moderate amount of heavily-implementation-dependent reflection (get the func type, construct arg(s), call func, check result), but that's about it.
Please just use values as the backing type. `type thing int` is utterly trivial to compare if needed, by comparison - just use `==`. Any code anywhere can construct the same argument in the same way and `==` to see if what they are checking is part of the args, and you're still not binding yourself to a specific implementation-type that'll later risk a compile-time failure.
(if users are casting to the private `int` type to check stuff, yea, it breaks - but the reflection-to-read-closure approach has that same problem, you can't stop it)
Also, in my experience, the "loop and apply" pattern tends to fall apart rather quickly, and you start wanting more complicated interactions between args / validation that you didn't pass both "include deleted" and "exclude deleted" and the second just silently clobbered the first. With values you can pretty easily loop and switch on the type and do whatever you need.
can you provide an example of a testing difficulty you’ve had?
func MyOption(arg int) Option {
return myoption(func(opts type) { opts.thing = arg })
}
MyOption(5) == MyOption(5) // false
Instead do:
type myOption int
func MyOption(arg int) Option {
return myOption(arg)
}
Whether you use the "loop and opt.apply(arg)" pattern or not doesn't really matter - that's a purely internal detail (personally I find it over-simplifies things and causes problems). Just "avoid passing values through func closures".
panic when there is a bug in your program; ie program not behaving as expected, all bets off.
return err when expected conditions fail: bad data, service down, whatever — handle gracefully.
I don't agree with this guideline. When you anyway have to remember to invoke an intrinsic function on the variable (unlike, say, operator overloading), I wonder why it's necessary to include yet another dependency that's just a wrapper for sync/atomic all because the developer doesn't pay attention to the right use of atomics.
Your comment (and others) have convinced me to do some more empirical testing and see how necessary buffered channels are for my goal.
The last sentence in the recommendation emphasizes this: use them with scrutiny.
Picking a higher queue size can increase the peak throughput. The queue will smoothen out peaks and valleys in the workload, and the other end of the queue will get rarer into situations where there is no work. If you know the duration of the "valleys" in your workload you can size your queue exactly to get over them.
However a too big queue size can easily lead to higher latencies. In extreme situations the work might be already outdated at the point of time it's processed by the receiver. And backpressure on the producer is less given.
Queue sizes > 1 can also mask concurrency issues (like deadlocks), which will then only show up rarely in production when the queue is fully exhausted. I guess that's one of the main reasons why they picked the 0/1 rule.
In my early React years, I had a lot of conversations about good vs bad as we got used to working in a declarative component hierarchy rather than the old imperative (tweak that DOM!) way. I still come across projects where people write html + bootstrap translated into a render method rather than using the power that is components.
In the "Reduce Scope of Variables", second good example leaks an open file when WriteString fails, because it doesn't follow the own advice of "Defer to Clean Up" if you are curious.
(Handling that properly with a defer is a bit more tricky - something like https://play.golang.org/p/l1PeWM3Tisg).
Update: style guide was fixed after this report :) It was this if you wonder: https://github.com/uber-go/guide/blob/a53ee0bef8c0b11b52340d...
"Styles are the conventions that govern our code. The term style is a bit of a misnomer, since these conventions cover far more than just source file formatting—gofmt handles that for us."
if err := ioutil.WriteFile(name, data, 0644); err != nil {
return err
}
Of course it's not the assignment which is being tested in the Go code above. But it might as well be.
In Swift, Rust, Clojure and Perl, "if let" is common. ("if-let" in Clojure, "if my" in Perl). It's useful, and conforms well to the idea of limiting scope of the tested variable. I use it all the time.
So scope limiting in "if" is a good idea. But in those languages, they have the benefit of a clear syntax with a keyword, and it's a single assignment+test operator, very unlikely to be an accident due to a typo, or to hide an accident.
In contrast, I think the Go snippet looks error prone because the actual condition is all the way off the right. The assignment is obvious and the intention to test it can be presumed when skimming the code. But because the test is way off the right, at a horizontal position which will vary in different code, and at the end of a long line with a compound statement, I think it will be easy when skimming code to fail to notice if the condition is wrong due to a typo.
So I'm surprised Uber recommends this one.