Go Style | Hacker News Reader

Go Style

google.github.io

327 pointstomcam205 comments

    > The general rule of thumb is that the length of a name should be proportional to the size of its scope and inversely proportional to the number of times that it is used within that scope. 

    > A variable created at file scope may require multiple words, whereas a variable scoped to a single inner block may be a single word or even just a character or two, to keep the code clear and avoid extraneous information.

I love how it covers both short iterators and long descriptive names under a single principle.

Good

Apple APIs are infamous for their long names. Not sure what they gain by it

I never minded it so much in Apple code; with the syntax for calling methods, it made method invocations sound like sentences, more like literal programming. Plus, intrinsic named parameters, so many languages - including Go IMO - would benefit from named parameters.

Never having to figure out what a function actually does is nice.

builder.makeThing("a", true, true, 46)

[builder makeThingNamed:"a" isRound:YES isRed:YES size:46]

or lately

builder.makeThing(named: "a", isRound:true, isRed:true, size:46)

Right? I have never understood the frustration around named parameters like this. It helps reduce cognitive load quite a bit.

It increases cognitive load when writing the function call, since you now need to remember what kind of true to use. It also increases cognitive load for people who either do already understand the parameters or who are doing something for which the parameters are not relevant; it's harder to ignore a long thing than to ignore a plain "true".

> It increases cognitive load when writing the function call, since you now need to remember what kind of true to use.

There is no "kind of true".

> It also increases cognitive load for people who either do already understand the parameters

By letting them see what the parameter they remember is? There's no cognitive load to seeing what you expect.

> or who are doing something for which the parameters are not relevant;

If the language doesn't have optional parameters, all parameters are relevant. By making parameters named, you avoid having to count positional parameters as in MS APIs to ensure you didn't get one in the wrong slot.

> it's harder to ignore a long thing than to ignore a plain "true".

Which is very much valuable. It's much easier to notice mistakes than when you've got 11 positional parameters of which 2/3rds are usually set to 0/null.

> Apple APIs are infamous for their long names. Not sure what they gain by it

Clarity.

It's also a factor of Objective-C's compound methods (inherited from Smalltalk), as each parameter names appears in the method, and which leads to a very phrase-like style.

By osmosis, the C APIs use a similar style.

This advice should be universal in coding. When I first started programming, I had a manager that hated 1-2 character variables. But they make sense for loop iterators.

For loop iterations it's fine, but I think Go code often takes this too far.

It often takes longer to read a single character than a word, because I have to mentally map the character to the word anyway.

Reminds me of when people go nuts aliasing table names in SQL queries, which IMO makes it harder to read as well.

I agree. Plus, with IDEs and auto-completion, why not use longer names?

One reason so use short names is to keep lines short and reduce line wrapping. This helps to read code, especially when you do 3-way merges.

- Longer names mean longer lines, and lines that are longer than your viewport are terrible. Less of a problem with modern ultrawide screens, but passed ~100 characters, it starts becoming a problem in some cases (ex: diffing). Personally, I have a soft limit at 80, hard limit at 120, because that's what works best for me.

- With such rules, variable length is a hint of its scope. For example, I tend to use "i" when the loop body is small, "idx" or "index" when it is a bit larger, and a more descriptive name when it exceeds one screen. This way, just by looking at a single line, I already have a hint about its context.

- Just because a variable name is short doesn't mean it is meaningless. For example "i", "j", "k", are loop indices, "x", "y", "z" are point coordinates and "dx", "dy", and "dz" are differences, "a" and "b" are both sides of a comparison function, "t" can be a temporary variable or a time depending on context, etc... The corollary is to make sure you use your single letter variables consistently. If I see an "x" in a place where a coordinate can be used and it does not refer to a coordinate, or an "i" that is not a loop index, I will be confused.

If you have to use that name a bunch of times close together, it bloats line lengths which also affects readability.

There's also a bit of Fitt's Law in here as well - I find it a lot harder to select, or put my cursor in, a single letter variable. So if I want to rename `i` to something else, I must carefully select `i`, whereas with a longer variable I find it easier to put my cursor in the middle of `index` and hit F2.

If you are using a mouse for things like this you already lost though.

Most developers use their mouse quite often.

I started using ii as my iterator variable name for similar reasons. Easier to highlight and Ctrl-F for. Also removing ambiguity when using i as the imaginary constant.

The constancy in Go makes this better. I have come to expect `r` to be an io.Reader or http.Request depending on context.

There are a few interfaces in Go that are used heavily and I don't mind that people often use a single character for them.

It's the same thing as everyone using `i` for iterators.

I just use reader//req. Code is instantly easier to read for me personally and no time has been wasted.

It's all preference, of course.

Sometimes you run into packages called reader which makes for a less fun time using variable names that are also words.

This is much nicer for other people coming into this. It's the same problem with spoken language and slang, slang is better if you know it, worse if you don't.

Sorry to be pedantic (well, not really it's M.O. for a hacker news commentor after all, isn't it?) , but as someone who knows a few languages, slang is not an issue with spoken languages at all. You learn a language by full immersion with other speakers, who impart that knowledge unto you. Unless all you are doing is conversing with grammaticians and newscasters, you will pick up on regional and local differences, euphemisms, and slang.

I don't believe so.

Imagine you're new to Go and you haven't mentally mapped all of these abbreviations. What would you rather read, `r` or `reader`?

Designing an API that "stutters" is a very common mistake that many programmers make. reader.Read() is pretty jarring.

There is also nothing wrong with naming variables after what they're for instead of what they are. Consider io.Copy(dst, src). That's nicer than io.Copy(reader1, reader2).

But not necessarily nicer than io.Copy(destination, source).

Sure, so call your reader `file` or `network` or `stream`. Don't call it `r`. Nobody is charging you by the character.

It isn't a huge deal when the code is simple, e.g. you create a reader named `r` and in the next line you use it. That's easy to understand. The problem is that the pattern propagates. A few more lines of code are added, the reader is still named `r`. It's name `r` here, so we start putting it in a struct as `r`. Soon enough the codebase is littered with instances of `r`. Not great.

This makes me wonder: what if there was a language where variable names are determined according to the type, with the option of overriding with a custom name. So a variable of type http.Request would automatically be named “req”, the next one in scope would be “req2”, etc.

If you think about it, when you solve a physics problem, for instance, you call every mass “m1”, “m2”, etc. Maybe this would be another step in Go’s direction of conforming style to make code more standard and readable.

This is interesting.

The idea of having a standard library of types related to what are effectively program keywords... could be really good. Or hideous like global vars.

I wonder if any language has tried this?

https://en.wikipedia.org/wiki/Hungarian_notation

Oh cool, thanks for that.

I agree that most types come with a natural variable name.

However, in many cases a more descriptive name is way appropriate. On top of my mind:

- Multiple variables of the same type. How are you supposed to distinguish between req and req2? Compare it to something like "apiReq" and "cdnReq"

- Primitive types, that does not inherently carry a domain value. An integer called "seconds" or "max_offset" has a lot more meaning that one named "num"

Forcing such a notation into the language would make impossibile to represent all these cases

The real question from this example is: why are you creating two http requests in the same scope before using them?

I've been writing Go cloud stuff for the better part of a decade and "req" for a request has never been ambiguous because I go ahead and send the request and process the response before sending another one, at which point I can just reassign the variable and let the first one fall out of scope.

The request type is just an example, of course, building up from the example in the comment I was replying to.

You’re right. Going back to my physics example, all the variables in a physics problem are actually of the same type, float. So you’d need more specific types, but that would be infeasible-the whole point of types in a general purpose language is that they’re general enough for any use case.

If you were just coding physics problems you could have types restricted to the physics domain, that is, physical units. Which it looks like MATLAB does now support: https://www.mathworks.com/help/symbolic/units-of-measurement...

Werrrrlllllll... It all depends on what yo umean by "type". In physics problems, I find taht quantities may be measured in "floats", but have the types "mass", "amperage", "distance per second per second" and the like.

In Go, I would probably model this as:

    type mass float64
    type speed float64
    type acceleration float64

That way, they'd all have float64 as the "storage type", but it would be harder to accidentally pass an acceleration when I wanted a mass.

> Going back to my physics example, all the variables in a physics problem are actually of the same type, float

This is why some people recommend a practice I've forgotten the name of, where they defined a new type that maps to language type. So type Mass could just be a float, but you know better how to treat it.

What if I introduce a new http.Request between 1 and 2? Now all the references are broken.

Terraform has an extreme approach to that, every time you reference a variable you have to spell out the complete TYPE.NAME.ATTRIBUTE, eg resource.aws_s3_bucket.my_bucket.website_endpoint. There is no way to access my_bucket without the whole prefix, even after assigning it to local variable you must prefix the reads as locals.my_bucket.

For sensitive infrastructure this makes sense, i surely wouldn't want to write other code like this.

That could be easily done by the editor in a statically typed language like Go, so there is no point bloating the language with it. Just turn on an overlay that displays the types of all names. I believe LSP adds this to many languages/editors already.

Or in languages that you can import a resource but give it a custom name. So whenever you are reading someone else's code you have to double check to understand what is going on. For instance, import DiskUtils as DU, and now all the code references DU.diskSpace()

Exactly.

I think a better heuristic rather than tying it to scope is to sort of semantically huffman encode. Using an iterator is probably the most used variable name that starts with an 'i', so it gets just plain 'i'. As a _concept_ gets more specific, then it gets a longer name.

Yeah i for for loops (in python) x for comprehensions, etc, make sense. But in general, single character variables make code really hard to read.

Yep, you should just use the best judgement, for example

    for distance, time in driving_metrics:
      speed = distance / time

is a lot clearer than

    for d, t in driving_metrics:
      s = d / t

even if it's used in a tiny scope once. But some things don't have a valuable meaning, or the meaning can trivially be inferred.

I meant i as in the iterator variable for a counter like when using enumerate() or a c style for loop where you manually specify the list index.

When I started coding I'd type

   10 FOR F = 1 TO 10 
   20   PRINT F, F*F
   30 NEXT F

Because "FOR" and "F" shared the same key, on my ZX Spectrum keyboard.

And for those that don't know - entering Basic code on the Spectrum (well the original one at least) was ALL single/shifted key shortcuts for keywords rather than typing them out.

eg https://www.old-computers.com/museum/photos/sinclair_zx-spec...

The low cost and quirky masochism was mainly why we loved it.

Same thing for the ZX-80 and ZX-81. If you were careful with your text prompts, you could save several bytes by injecting a keyword instead of typing it out, but anything that was a "start of line only" could be tricky injecting into a string.

The more I think about this rule the more I like it.

One thing I would add is if you're writing a library, think of the people using it. Are they going to curse you every time they have to write YourModule.ExtremelyVerboseMethodNameWhyOhWhyIsItSoLong, even if it only appears once in your module?

As soon as you start sharing code, you can't know how many times a name will appear. So err on the side of brevity, for our sakes.

> A variable created at file scope

How does one declare a variable to be of file scope in Go? An ordinary "var x int" has package scope, visible from any file in the directory (aside from the special cases around _test.go files).

Scope of package import declarations is narrowed to a single .go file, but these of course are not variables.

I think you’re reading this too literally. They likely just mean file scope from a purely usage perspective. I.E. you only use the variable in one file, it doesn’t matter how the language processes it into a computational scope

It absolutely does matter. If I'm editing the file in which the package-scope variable (or type, or const) is declared (and also referenced) and decide on the fly to subtly change its semantics, forgetting that the same is also referenced from some other file within the package, I have likely just created a bug. Worse, the bug will likely manifest itself only at run time.

You're still taking things too literally.

A human typed in something less precise than you'd like in a style guide. Find a bug tracker to request a verbiage change. It's not that big of a deal.

No one is asserting that variables are in fact file scoped.

> Go interfaces generally belong in the package that consumes values of the interface type, not a package that implements the interface type. The implementing package should return concrete (usually pointer or struct) types.

I like this rule. Most companies violate it everywhere. There are good times to ignore it but I always push for

  func NewThing

To return something other than the interface type. The last Go interview I was in I brought this up and the argument was that you cant test things if you are not returning the interface and I disagree entirely.

I love the power of duck typing with interfaces but give me concrete things that quack.

Yep - return concrete types, but also make sure you have an assertion along the lines of:

    var _ IfaceType = &ConcreteType{}

Somewhere, else you risk not knowing that a change to ConcreteType broke is implementation of IfaceType in a way that you might not know about until a rare runtime code path is executed.

> Somewhere, else you risk not knowing that a change to ConcreteType broke is implementation of IfaceType in a way that you might not know about until a rare runtime code path is executed.

Why would runtime be involved? Surely if ConcreteType doesn't satisfy the interface anymore then the compiler will catch that at any site where a ConcreteType is being used as / cast to an IfaceType?

Or are you talking about iface->iface side-cast?

The compiler will catch it at the point of use, but it's clearer if the error is at the point of declaration. Consider:

   type FooReader struct{}
   var _ io.Reader = FooReader{}

   func (r FooReader) Raed(p []byte) (int, error) { return 0, nil }

Before you even use FooReader as an io.Reader, you can find that you typo'd "Read". This is useful in practice because you probably wrote FooReader to have a bunch of other methods that aren't implementing io.Reader, and it's possible that your tests for this object don't actually ever use it as an io.Reader. So you won't notice this mistake until someone tries it elsewhere in the codebase.

It also acts as documentation that you intend for this thing to be an io.Reader. But I also recommend that you document Read as "// Read implements io.Reader." to be extra explicit.

Hah, how’s that for an endorsement of structurally typed interfaces.

That rule is not without its issues when pointers get involved, due to the typed nil problem.

Yeah, this is a setup for hard to understand bugs unless you have a corresponding rule (imposed by a linter) that a concrete type cannot be cast into an interface and then checked against nil (mostly this means when returning a concrete type from a function it should be assigned to a freshly declared variable rather than re-using a variable that could already have an interface type.

In practice this never happens unless you're violating other critical best practices like explicitly ignoring errors from constructors and trying to use the (nil) returned value anyway.

I think it doesn’t happen for errors because the error interface is almost always what is returned. It doesn’t seem that hard across if this style guide were followed: https://news.ycombinator.com/item?id=33566422

Typed nil isn't a problem, it indicates a crucial lack of understanding about interfaces, which should be resolved with education, not restricting good API design.

Interfaces are the most misused feature of Go. I think people like to prototype their API by typing in all the functions they intend to implement, so the interface acts as a template for their program that they can fill out. This, unfortunately, is not what they're for.

I have a longer rant about this here: https://jrock.us/posts/go-interfaces/

Thank you for sharing this.

I was struggling with this just today: Realizing I keep putting off writing tests for some modules because mocking a certain mega interface is a pain.

Your post gives me some great hints for things to try. Looking forward to experimenting with it.

It took me a long time to figure out how to make small interfaces. I had an "aha!" moment not too long before writing that post. It seems bad, then it suddenly makes sense, and then the other way seems bad.

Adding private members to my structs for testing was something the Go team forced me kicking and screaming to do when I worked at Google. I remember writing this adaptor for some internal network filesystem, and had an interface and two implementations: one that used the network for real, one that did everything in-memory for tests. I sent it off for code review and they were like "nope! don't do that in go!" and suggested an if statement in every method to handle the in-memory implementation. I was unhappy about it for months because it went against everything I knew about programming at the time, but like 10 years later, I appreciate that they were right. (I will say that everyone I explain this to has approximately the reaction that I had at the time. It is an easier pill to swallow when the person telling you not to do it wrote Go, but they don't have that benefit ;)

Really interesting approach. Thanks for sharing. Do you remember by any case where to find any real example? I'm digging into Golang's repositories trying to find one...

Wow, I find this quite interesting, however, there are probably some real downsides to this? Maybe someone else can highlight some of those.

An issue I can think of is for instance: consider a system where each time a new request comes in a new transaction is started. That situation would result in having a new `DBImpl struct` for each request. It seems that that last "but you probably only have one of these DB objects" doesn't hold. How would you tackle that issue?

Sure, if you want to compose the various methods into a single transaction, then each business-logic-implementing function needs to take a transaction object as a parameter. In that case, you don't even need something like DBImpl.

Looking at database code I have written, I never do the object thing (but was replying to an author who does). I typically have a package with functions that take transactions as an argument, like this:

https://github.com/jrockway/jsso2/blob/master/pkg/store/sess...

(If you poke around the package, methods that don't make more than one database call take a sqlx.ExtContext instead of a *sqlx.Tx. Functions that need to start their own transactions take the connection instead, though I'm not sure I'd recommend that approach because it hurts composability.)

To test it, I just run against an actual Postgres instance which is pretty easy to find. (I am not sure I would steal my database setup / teardown utilities from that project. The tests end up indented too far. A more idiomatic Go way is "app := testapp.New(); defer app.Cleanup(); tests go here"; in that codebase I picked the "testapp.Test(t, func(t testing.TB) { tests go here })" pattern. Definitely something I look for and avoid these days, but didn't in 2020 ;)

Thanks for your reply, very interesting, I didn't know about the `sqlx.ExtContext` interface yet! I will do a deeper dive into your project later, I'm hoping to find more inspiration.

Slightly unrelated to Parent:

I struggle very hard creating my own interfaces for my own programs. I haven't found any literature online teaching the _right way_ of coming up with your interface.

For example, I very often struggle with function parameters and return types: should my interface functions only take basic type and return basic types? Can my interface function take more concrete types? Should my interface function take interface types?

I'd love to be directed to a guide on how to create a good interface.

As an example, here's a recent interface declaration I wrote:

    type Checker interface {
        Check(context.Context, *object.Commit, bool) (bool, []string, error)
        Name() string
        Describe() string
    }

My program runs through a list of commits and runs "Checks" on them. These checks "pass (true)" or "fail (false)".

I want the user of my package to be in charge of the implementation details of a "Check", so I created the interface above.

Is it OK for the Check() function to take a *object.Commit? Should I instead pass a `SHA string` and let the implementer figure out how to get a *object.Commit from that string?

Looking up *object.Commit from SHA string doesn't seem like something you want to make every Checker implementation do (even if it's mostly boilerplate), so passing a higher-level object makes sense here.

What you might want to do is also define an interface for what Git calls "commit-ish"[0] objects, and use that in place of *object.Commit. Then you could pass e.g. *object.Tag in place of *object.Commit.

Think of the interface as a contract between implementations and users of the data structure. Typically, you use an interface when multiple implementations are capable of fulfilling the same contract. If there's only one implementation, you likely don't need an interface—for instance, you can ignore my suggestion about abstracting commit-ish if you're pretty sure your checker would never be called for a tag.

Interfaces are often useful when testing, because a mock implementation can fulfill that same contract. For instance, you might implement a *object.MockCommit which just uses static values instead of actually operating on a Git repository, and then pass that for your commit-ish in unit tests for various Checkers.

[0] https://git-scm.com/docs/gitglossary#Documentation/gitglossa...

I prefer to make the implementation package-private and hide it behind an interface if there's no way to make the implementation's zero value useful. Doing so prevents someone from trying to use it without calling the (mandatory) constructor.

My favorite Easter egg when helping to write the style guide was including a reference to Full Metal Jacket within it. See if you can find it.

In all seriousness, I am very proud of the team’s work in both writing and publishing this externally. It was a lot of work.

I found the following statement in the Maintainability section interesting:

> Maintainable code minimizes its dependencies (both implicit and explicit). Depending on fewer packages means fewer lines of code that can affect behavior. Avoiding dependencies on internal or undocumented behavior makes code less likely to impose a maintenance burden when those behaviors change in the future.

Obviously this guide was written for internal Google use, but it's interesting to see a recommendation that appears to run a bit counter to how ecosystems tend develop for languages that have easily consumable packaging solutions. It's not uncommon to look at a Go, Rust, Node, Python, etc project and see many dependencies (direct and indirect) in use, most of which you have no idea what they're being used for outside of some of the most popular packages. Not that I think the suggestion is wrong, I fully support fewer dependencies and using external packages where they make sense. I start to get uneasy when you see a tree of dependencies of unknown code, but I don't have the time myself to read through all of those packages.

A lot of this comes down to available resources too. I used to hate how many third party things we used on principle and because I directly felt all the pain of tying them together. Now I’m a lead and I still dislike it but because I have a realistic read on how much bandwidth my team has I think this has balanced my view. I also still feel the pain but sometimes more indirectly (code reviews, for example).

I think where I still struggle is “rules for thee but not for me”. At some point, someone on the team (usually the person in charge) is just going to be better at picking which external libs to let in. I find this really hard to teach without it seeming arbitrary. I find it a lot easier to teach good coding habits.

This works with high quality or high familiarity libraries, which usually stem out of doing something very simple, but as I've gotten more experienced I've learned that most libraries it's faster and clearer to write the one or two functions I need myself instead.

I imagine that opinions such as this have influenced this recommendation: https://research.swtch.com/deps . I think there is some spirit in Go of not taking on a ton of small dependencies, but that may be a hold-over from before Go had a built-in package manager.

I think as an organization grows to some combination of available resources and severity of an outage this view becomes more and more common.

There's definitely a balance to strike.

On the one hand, you don't want to be constantly re-inventing the wheel. If someone has made a great package for doing a specific thing you need (printing data in tables, making ergonomic http requests that cover edge cases, reading an epub file, etc), then IMO you're better off using theirs. They care about it, they've tested it, and the implementation of it is one less thing you have to think about it. If you consider that every line of code that you write/maintain is a liability, then offloading that to others is very convenient.

On the other hand, external packages typically come without guarantees. Their owner/maintainer could lose interest, get hit by a bus, get hacked, anything. You're running their code in your projects and suddenly, all this uncontrolled code becomes the liability. You'd do best to do _everything_ in house in order to limit your external risk.

Ultimately, I think there's no single right answer here. Sure, you probably shouldn't add dependencies for things like "left-pad", but the line is a little blurrier for things like npm's "is-promise". It sounds simple enough (and the implementation [0] is only a 3-line function), but I'm unlikely to have written it correctly if I did it from scratch. Plus, it's tested! And lastly, I think your risk is lower the larger an external dependency is. Large projects, like React or Django, are much more upside than liability; it's everything in the medium range that you really need to consider.

[0]: https://github.com/then/is-promise/blob/ec9bd8a3f576324a1343...

> Name constants based on their role, not their values. If a constant does not have a role apart from its value, then it is unnecessary to define it as a constant.

This breaks the linter in most cases because "magic numbers". Like having to declare a constant for the number of cents in a euro/dollar.

I think Google are right in this case, and linters need to be smarter.

> Like having to declare a constant for the number of cents in a euro/dollar.

I agree with your point, but this is a bad example. Naming these constant CentsInEuro and CentsInUSDollar is consistent with the style guide.

As silly these examples are in isolation (of course a cent is 1/100 of a euro or a dollar), if you are writing code that processes currencies beyond euro and dollars, you will quickly end up with (using Chinese Yuan as an example):

    const (
        JiaoInCNYuan = 10
        FenInCNYuan = 100
        ...
    )

At which point adding

    const (
        CentsInUSDollar = 100
        CentsInEuro = 100
        ...
    )

is not just reasonable but a good idea.

Additionally, suppose your code needs to deal with historical currencies, you'll most likely need:

    const (
        ShillingsInOldBritishPound = 20
        PenceInOldBritishShilling = 12
        ...
    )

This is also a good example that the knowledge that a cent is 1/100 of a dollar or euro is highly culture-dependent. A British programmer from as late as 1970 will tell you that it's silly to create constants for these: of course there are 20 shillings in a pound and 12 pence in a shilling, everyone knows that! (I also like to think that they'd assume that a "cent" is 100 dollars because that's what centum means in Latin, but it's probably highly unlikely for them to never know US dollars and cents.)

Yeah maybe. I've worked on code where there were constants defined for minutes-in-hour, hours-in-day etc though, and that was just annoying; it's conceivable that our culture will one day decide that the Babylonians were idiots and we should use the French revolutionary clock instead, but I'll take my chances

More realistically, software will run on other celestial bodies which have different time periods (e.g. the moon, mars, asteroids, etc.).

For me, the use of those annoying constants is a form of mental relief: I don't have to remember why I'm dividing by 60 or 100 or whatever in this specific spot, it's written out in a way that reading it provides the context.

How many seconds in an hour? Most of the time, 3600, occasionally 3601, and very rarely 3599. Hours in a day? Mostly 24, but 23 once a year ad 25 once a year.

These all seem like good reasons to make then functions (taking a timestamp as a n argument) rather than mostly-correct constants.

I swear, the more I learn about calendars and timekeeping, the more I realise I never ever want to deal with it.

Tell the reader that this here code is prepared for leap years, leap seconds, leap hours:

  SecondsPerStandardHour = 3600 // or NormalHour or TypicalHour 
  HoursPerStandardDay = 24      // or NormalDay, TypicalDay

ok, fair point, bad example. But having to declare a constant for the number of percentage points in a unit is insane and needs to stop. Bad linter, bad.

> A little copying is better than a little dependency.

Go takes this to an extreme, though. Generics helps, but the implementation is so limiting that it doesn't help much.

> The general rule of thumb is that the length of a name should be proportional to the size of its scope and inversely proportional to the number of times that it is used within that scope.

Some (most) of the go code I've seen where I work suffers from Enterprise Java style identifiers. That is, TheyAreGenerallyTooLongAndIHateThemAll.

This is probably because Go doesn't allow function overriding. You can't have `streamFromUrl(foo)` and `streamFromUrl(foo, bar)`, so you end up with `streamFromUrlWithFoo(foo)` and streamFromUrlWithFooAndBar(foo, bar)`.

Consider variadic arguments for such cases.

There is a group of developers where I work who would take a perfectly fine `New(…*Options)` constructor and write a whole suite of “option wrapper” “utilities” around it, rather than expose that function. It makes me tear up a little (not in a good way).

No doubt: and that is generally the pattern I’ve seen. It’s really an eyesore.

Right. It's kosher to have one simple "New(..)" function per package, for the key type in the package, but all the other constructors shall have verbose names.

"A little copying is better than a little dependency" is not talking about anything that would be solved by generics or metaprogramming, which suggests that you are completely misunderstanding. It's saying to not import a large library if you only need one five line helper function from it. Generics do not help, and the advice could be applied to any programming language.

The kubernetes ecosystem has a lot of go code which consequentially suffer from nil panics. Thankfully they recover otherwise we'd see an absolute shit ton of pod restarts.

In general, please stop panicking in library Go and Rust code. It's rude.

Totally agree, panic in a library is a smell

nil panics are hard bugs, not stylistic concerns. There is never an occasion where correct code dereferences a nil pointer.

The style point that people are missing here is that every type should have a usable zero value. If (&SomeStruct{}).Method() panics, that's an API design flaw.

Yes! This is one of the tiny few things I don‘t like about rust. In a language so focused on safety, why can anything panic? Every single failable function should return result. No ifs or buts.

In my opinion the hardest style rules to accept when trying to use this guide are:

  1. Do not create "assertion libraries" like `assertEqual(x, y)` [1]
  2. Leave testing to the Test function [2]
  3. Intialisms (HTTPURL, IOS, gRPC) [3]
  4. Function formatting [4]

For the record I'm not saying I disagree with these. I just think that folks coming from other languages have a lot of built in muscle memory to do it other ways.

  [1] https://google.github.io/styleguide/go/decisions#assertion-libraries
  [2] https://google.github.io/styleguide/go/best-practices#leave-testing-to-the-test-function
  [3] https://google.github.io/styleguide/go/decisions#initialisms
  [4] https://google.github.io/styleguide/go/decisions#function-formatting

Yeah IMO assertion libraries are ok if you actually agree with your colleagues to provide context for every check in the assertion message. Obviously that means that you have to actually have that conversation, but it's no different from the advice to just never use those libraries either.

I've read the assertions section a few times and I still don't understand the argument. How is:

  if got == nil {
    t.Errorf("blog post was nil, want not-nil")
  }

Better than

  assert.NotNil(t, got, "blog post")

? They seem to suggest that you lose context, but their "Good" examples are similarly devoid of context.

The main reason I dislike the assert libraries is that you lose the ability to format things nicely; being able to see what's going on quickly is pretty nice when tests fail. Sometimes you want %s, sometimes %q, sometimes maybe a diff or formatted as something else. Testify "solves" this by just dumping a lot of stuff to your terminal, which is a crude brute-force solution. Plus with table-driven test you're saving what, 2 or 3 lines of code?

For example for a simple string comparison it's 12 lines:

   --- FAIL: TestA (0.00s)
       --- FAIL: TestA/some_message (0.00s)
           main_test.go:11:
                   Error Trace:    /tmp/go/main_test.go:11
                   Error:          Not equal:
                                   expected: "as\"d"
                                   actual  : "a\"sf"
   
                                   Diff:
                                   --- Expected
                                   +++ Actual
                                   @@ -1 +1 @@
                                   -as"d
                                   +a"sf
                   Test:           TestA/some_message

vs. 2 lines with a "normal" test:

   --- FAIL: TestA (0.00s)
       --- FAIL: TestA/some_message (0.00s)
           main_test.go:11:
               have: "as\"d"
               want: "a\"sf"

With your NotNil() example it's 4 lines, which seems about 3 lines more than needed.

This kind of stuff really adds up if you have maybe 3 or 4 test failures.

NotNil is fine, but as a library author you need to implement every comparison operator for every type. You also have to implement each assertion twice; once for t.Error and once for t.Fatal. We have a homegrown assertion library in our codebase at work, and every time I write a test I have to settle for an inaccurate assertion, or add two more methods to the library. I grew up on Go at Google where I would not have been allowed to check in assertion helpers, and I think they made the right call there.

I've seen a lot of gore in code that uses assertion libraries like assert.Equals(t, int64(math.Round(got)), int64(42)). Consider the error message in that case when got is NaN or Inf.

It is so easy to write your own assertions. I can type them in my sleep and in seconds:

    if got, want := f(), 42; got != want { 
        t.Errorf("f:\n  got: %v\n want: %v", got, want)
    }

    if got, want := g(), 42.123; math.Abs(got - want) > 0.0001 {
        t.Fatalf("g:\n  got: %v\n want: %v", got, want)
    }

Why implement a NotEquals function when != is built into the language?

(The most popular assertion library also inverts the order of got and want, breaking the stylistic convention. It's so bad. Beware of libraries from people that wrote them as their first Go project after switching to Go from Java. There are a lot of them out there, and they are popular, and they are bad.)

Finally, cmp.Diff is the way to go for complex comparisons. Most use of assertion libraries can be replaced by cmp.Diff. I wouldn't use it for simple primitive type equality, but for complex data structures, it's great. And very configurable, so you never have to "settle" for too much strictness or looseness.

Coming from Swift development I first missed having a collection of assertion functions for testing, but I've come around to the go testing patterns. I do think test assertion libraries usually result in less useful messages, and you end up having to implement a new function for every type of comparison under the sun.

(One thing I absolutely abhor is those assertion DSLs similar to rspec)

Drawing inspiration from:

> Complex assertion functions often do not provide useful failure messages and context that exists within the test function.

I think the best compromise is to avoid combining individual logical units into one assertion. For example:

Bad:

  assert.True(t, myStr == "expected" && myInt == 5)

Good:

  assert.Equal(t, "expected", myStr)
  assert.Equal(t, 5, myInt)

Real world example: at my workplace we have some code that tests HTTP request formation for correctness (right URL, body, headers, etc). Replacing big all-or-nothing booleans with individual assertions on each property of the request provides much more useful test failure messages.

As with any published "best practices" like this, have an open mind but don't just cargo cult whatever Google does. Best to be selective about what does and doesn't work for your situation.

If you want context on why some testing guidance is the way it is, this external discussion may be helpful: https://www.reddit.com/r/golang/comments/yy8edb/googles_inte....

<pet_peeve> [3] is an Americanism, innit ? They'd rather write EBCDICBlah than EbcdicBlah. And in the same vein, W.H.O. instead of WHO, and 8:30 p.m. instead of 830pm. </pet_peeve>

Kinda amusing how the following section is complicated by the language's casing-based visibility feature

https://google.github.io/styleguide/go/decisions#initialisms

Indeed, this is one Go convention that I strongly dislike. Java did have that approach once (HTMLDOMURIReference, XMLIDREF and the likes) but they learned their lesson.

And what's the basis of capitalizing D in ID?

Because "ID" is short for "Identification Datum".

citation needed

It is commonly abbreviated that way in normal life (see https://en.wikipedia.org/wiki/Identity_document ) When I see "Id" in code, I wince.

Rage. Raging Id.

I also dislike "ID" vs "id" (as short for "identity"), but there is some case for it. The below are from Webster's Third New International Dictionary Unabridged.

First, the case for "id":

    id abbreviation  
    1 [Latin idem] the same

Now for "ID"

    ID noun, plural ID's or IDs [REVISED] [identification]
    1 a : documentation bearing identifying information

And "Id" is that Freudian thang, not quite the same as identification.

I like this. Generally speaking, information is not lost in this form—as in form does not change for initialisms. We have the casing on purpose in English to communicate that you are looking at an initialism or abbreviation so why would you lose that to change in variable names? Obviously there are some cases this doesn’t work as many languages variable names need to start with a lowercase letter, hence the need for the section.

In practice it's not an issue and is quite sensible/intuitive how it works, at least for English speakers.

golang driving me nuts these days. enjoying the performance, but time.Parse put me into a ragequit mode last night and I really wish it was possible to return a thing OR nil

Why not writing a small wrapper around time.Parse?

  func ParsedTimeOrNil(s string) *time.Time { 
   t1, err := time.Parse(time.RFC3339, s)
   if err != nil {
    return nil
   }
   return &t1  
  }

This is the answer. Write a utility instead of get frustrated with the language; be pragmatic. Same with looking for libraries that do something trivial.

I’m used to parsing a date time with things like %Y-%m-%d not the asinine way Go does it.

If you're using JetBrains Goland, hit Ctrl+Space when you're writing the date layout in `time.Parse`, and it'll suggest you the regular YYYY, MM, dd etc. placeholders. When you pick one, it types in 2006, 01, 02 magic numbers. Here's a GIF:

https://imgur.com/FyCJZvh

That said, I hate this, all the ways it's opinionated and praised zealously for it as the best thing since sliced bread.

Hiding errors like that is going to make it hard to figure out what is going on as more and more wrappers like this pile up.

If time.Parse is failing, you either have bad input or a bug, right? If you are ok with this failing without even logging the error, something might be wrong with the design.

Hiding errors saves less than a second of typing, at the cost of making every five minute bugfix a one day bugfix. Don't do it.

You can't build your own programming language inside of Go. If you absolutely cannot mentally handle functions returning an error and you having to type "if err != nil { fix the problem }", you really need to find a different programming language. But, errors happen all the time, and handling them correctly is the difference between an unreliable piece of garbage that randomly fails and software you and your users can trust. There is, unfortunately, no automation around making software reliable.

Plus, even if you just return an error/bubble up an exception, that is probably better than bubbling up a nil/null value for some of the consuming code to try to dereference.

Bubbling up is a great option. It is such a good opportunity to capture relevant context that the caller doesn't know about; like the internal state of the method, and the "why" behind making the call. If everyone does this, you can end up with an error message like "handle /ping: health check servers: ping server 1.2.3.4 (3/42): i/o timeout" instead of just "i/o timeout" (which you get if you just lazily "return err"). You see the first error message, and you know you need to adjust the ping function to treat a timeout as a part of the response. You see the second one, and all you can do is scratch your head. It could be anywhere.

(BTW, stack trace proponents... stack traces don't capture loop iterator variables, or "why" you're calling a particular function. But when you write a quick error message with fmt.Errorf, you can include all of those things.)

Software engineering is a job of continuous improvement. Make it really easy to find the right place to target improvements!

You can make your own Maybe with generics.

And why isn't this build-in since the beginning like in any other sane language?

A language where you need to constantly write wrappers around everything just to get basic functionality is quite a miserable language, imho. That's like C…

Completely agree.

> A language where you need to constantly write wrappers around everything just to get basic functionality is quite a miserable language, imho. That's like C…

Which makes complete sense, since Go was designed as Google's C for Dummies.

I know. Even by the same people that like to repeat their mistakes.

>Concise Go code has a high signal-to-noise ratio.

A few lines later:

  // Good:
  if err := doSomething(); err != nil {
      // ...
  }

"Tell me, how many lights you see?"

I think that it is fair to say Go considers error handling to be signal, not noise.

This position has obviously prompted a philosophical war and not everyone agrees, but the two statements are consistent under Go's philosophy.

What's wrong with this? The code is doing... exactly what it says. Call doSomething and if it returns an error do something else. What part of that is noise?

As does most code. That doesn't mean it has a high signal:noise ratio.

Some of these threads seem like that party game where people write a story together, but each player can only read the previous sentence of the story when writing the next one.

Best practices should differentiate between libraries and applications. Otherwise, like this guide you assume every piece of code is a library and slow down application development.

For example they recommend not using %w for errors- because library authors need to be careful about what information is exposed. However, as an application code developer, %w should be used by default- this avoids an accidental bug where annotating an error could cause an upstream error check to fail- which they allude to in the guide- but now their rule is getting complicated whereas "default to %w" would be safer and improve velocity in application code.

Go is absurd. It's opinionated in all the wrong ways.

> Functions that return something are given noun-like names.

> // Good: > func (c Config) JobName(key string) (value string, ok bool)

> A corollary of this is that function and method names should avoid the prefix Get.

> // Bad: > func (c Config) GetJobName(key string) (value string, ok bool)

That's dumb. I'd like a function to be GetJobName to indicate that it doesn't mutate anything. Maybe CreateJobName to indicate mutation. Just JobName is useless.

The other day, I spent a whole day trying to figure out the "idiomatic" way to return a an object not found case from my db. Do you return a nil pointer (don't, passing pointers leads to bugs), or an empty struct (then how do you reliably "test" its emptiness?) or an error? And of course, there's no real hierarchy of errors, no built-in extensible handling of errors that is semantic and makes sense, so every project just goes and reinvents their wheel.

If you rely on naming function to know if things can be mutated you're doing things wrong. The only source of truth is the type received.

Why? Let's say I have a table full of cars and have a method

> func Car(id string) Car

How do I know whether it fetched that from the database or created a new one and returned that to me?

Only the type Car matters, why would the function name tells you anything about mutation? It does not tells you if you receive *Car or Car or if it's a Car but pointers inside.

Because I want to know what function does?

I think it's not possible to achieve with func name on its own, the closest thing I see would be comments on the function but again not reliable imo.

Absolutely not reliable. Comments get stale faster than anything else in the codebase.

Assuming Car() is a receiver function, you should be able to infer "how" the Car is "fetched" based on the type receiving the function call.

func (d CarDB) func Car() (Car, error)

The above tells you everything you need to know. As for usage, again, the type and now also the variable names should let you infer everything you need.

func f(db *CarDB) { c, _ := db.Car() }

The function name makes as little of a guarantee as to the underlying "how" as these other factors.

I mostly avoid getters/setters in Go code, unless I have a lot of functions all related to the same thing similarly named. It's the return type that denotes the return. It's already in the function signature.

That advice is a consequence of Go expecting function signatures to be read and understood. That's not the right choice for all programming languages, but for Go it works.

I mean, you can't get around database CRUDs. I'm not talking about setting a private variable and getting or setting it (that doesn't seem like idiomatic Go at all), but using the logic described above, you can't know whether a method with a noun-based name is a Read or a Create.

Shouldn't a Create have the form NewBlah ? Incidentally warning that there is a memory allocation.

> I mostly avoid getters/setters in Go code, unless I have a lot of functions all related to the same thing similarly named. It's the return type that denotes the return. It's already in the function signature.

But I thought Golang doesn't have function overloading, so you also have to name the function appropriately for its use?

Yupppp. But you can't let function names get too long or everyone will laugh at you for being a Java dev writing Go.

Like I said, absurd.

Just assume that "Noun()" gets the noun and it's effectively the same as "GetNoun()". Of course, nothing in the language prevents you from using "GetNoun()" if you strongly prefer that.

> The other day, I spent a whole day trying to figure out the "idiomatic" way to return a an object not found case from my db. Do you return a nil pointer (don't, passing pointers leads to bugs), or an empty struct (then how do you reliably "test" its emptiness?) or an error?

sql.ErrNoRows is often used for this.

One more cognitive jump to get to the point. Programming languages are for people, so reducing any mapping between what is written and what is meant is for the better. Adding more jumps, however easy, is rarely the right thing.

It's just what you're used to. Anyone can make up some pseudo-scientific "cognitive" gobbledygook ("fewer words mean less language processing cognitive overhead"), but it really is just a matter of "what you're used to".

> That's dumb. I'd like a function to be GetJobName to indicate that it doesn't mutate anything. Maybe CreateJobName to indicate mutation. Just JobName is useless.

It can't mutate anything with a non-pointer receiver. Mostly.

> how do you reliably "test" its emptiness

See time.IsZero() for an example.

> If the receiver is a “large” struct or array, a pointer receiver may be more efficient. Passing a struct is equivalent to passing all of its fields or elements as arguments to the method. If that seems too large to pass by value, a pointer is a good choice.

For an app that has medium-sized structs in hot paths, this tradeoff introduces considerable tension into the development flow.

However, there is one takeaway for us devs: Don't count on help from the inlining optimization pass of the compiler to evaporate away calling overhead of structs, neither when passed as the receiver nor as an ordinary arg.

Isn’t the point of go is that go fmt follows the languages global style guide?

Go's fmt enforces some parts of formatting, like indentation, spaces vs. tabs and spacing around brackets and parentheses. But it is by no means a complete style guide. Fmt doesn't give your packages, structs or methods reasonable names automatically. Fmt doesn't tell you where to use values and where to use structs, what your interfaces should be, and how large your functions should be.

There is still room for a style guide.

Formatting ⊊ Style

Style may also include naming, documentation requirements, recommendations regarding function length, etc.

It includes things that are not formatting specific like naming (https://google.github.io/styleguide/go/best-practices)

At first glance, that question makes sense; however, fmt doesn't cover all code format issues. Ex from the post:

> Line length

> There is no fixed line length for Go source code. If a line feels too long, it should be refactored instead of broken. If it is already as short as it is practical for it to be, the line should be allowed to remain long.

> Do not split a line:

> Before an indentation change (e.g., function declaration, conditional)

>To make a long string (e.g., a URL) fit into multiple shorter lines

fmt doesn't force arbitrarily short lines in lieu of readability (looking at you python + pep8; I can't recall how many lines of code were a few characters long and pep8 formatting made the multiple lines a mess to visually parse). The Google Team decided that it was important to not break up perfectly readable lines and gave guidance on how to do that.

I hate autoformatters/linters that frob with line length with a passion. I'm an 80-column kind of guy, but 81 columns can be just fine, and in some cases 90 or even 100 can be more readable than obsessively wrapping at 80 (or some other arbitrary value).

There are a few (rare) cases where gofmt will wrap lines, but it mostly leaves it alone which is one of the better "features" IMHO. Many *fmt tools really got this wrong.

Automated formatters are nice, but in the end there is no substitute for human eyes and common sense.

I hate this about default rustfmt.

Yeah, and that allows them to just write "all Go source files must conform to the format outputted by the gofmt tool" under "Formatting" and move on to the stuff for which a style guide is still needed...

That is just a formatting guide. How to write spaces, newlines, semicolons.

Go fmt doesn't change anything that is not formatting.

Yet.

Yes - I always thought that Go took an opinionated position that the style is whatever gofmt outputs. This is a very limited set of style "rules" - that is on purpose. gofmt is meant to allow one to spend near zero time manually styling code or picking people up on styling in reviews.

Interesting that Google uses GitHub still. With Microsoft owning it and it becoming less free than alternatives, you'd think for documentation like this they'd use an externally hosted GitLab instance for their external projects or that they'd have acquired something to compete with GitHub by now like Gitea.

They could call it "Google Code" /s

https://gerrit.googlesource.com/

It is nevertheless the case that the Go Style Guide and all the other style guides were officially hosted in the past at https://google-styleguide.googlecode.com/ and now that's a 404.

Less free? Microsoft didn't change a damn thing on GH, in fact its only getting better after acquisition.

Another case of the lack of distinction in English between "libre" and "gratis" causing problems, I see.

has anyone compiled a comprehensive list of style guides from leading companies?

google has published a style guide, but it doesn't seem like facebook and netflix do.

it would be awesome to have one central list of best practices from leading tech companies.

we started one on github, but it's woefully limited: https://github.com/HotpotDesign/Developer-Style-Guides

could anyone kindly recommend better ones?

https://github.com/uber-go/guide/blob/master/style.md

Somewhat off topic, but what template does this style guide use. Very clean, want to use it myself. Any similar ones also appreciated!

> Go source code uses MixedCaps or mixedCaps (camel case) rather than underscores (snake case) when writing multi-word names.

> This applies even when it breaks conventions in other languages. For example, a constant is MaxLength (not MAX_LENGTH) if exported and maxLength (not max_length) if unexported.

Good. All-caps for constants would make no sense in a language like Go.

> Go tips - stay tuned

I'm looking forward to this document.

I found this "best practice" curious to read:

> The standard net/http server violates this advice and recovers panics from request handlers. Consensus among experienced Go engineers is that this was a historical mistake. If you sample server logs from application servers in other languages, it is common to find large stacktraces that are left unhandled. Avoid this pitfall in your servers.

I don't think I've ever seen a server library — HTTP or otherwise — that didn't have a top-level "catch all exceptions" or "recover from panic" step in place, so that if there's a problem, it can return 500 (or the Internal Server Error equivalent) to the user and then carry on serving other requests.

My reasoning is that any panic-worthy programming error is almost certainly going to be in the "business logic" part of the server, rather than the protocol-parsing "deal with the network" part, and thus, recoving from a panic caused by processing a request is "safe". One incoming request could cause a panic, but the next request may touch completely unrelated parts of the program, and still be processed as normal. Furthermore, returning a 500 error but having nobody read the stacktrace is bad, yes, but it's way, way, way better than having your server crash meaning nobody can use it ever.

Oh wait, is the assumption here that your service is being run under Borg and has another 1000 instances running ready to jump in and take the crashed one's place? Is this another case of Google forgetting that people use Go outside of Google, or am I reading too much into this?

If your program fails in testing, and no-one reads the logs, does it ever get fixed?

There is one advantage to not having a top-level catch: if it fails in testing, it's very obvious immediately.

Though I do agree with you - and this can be done with a feature flag for dev/prod servers.

The Echo web framework is an interesting example of this [0]. By default it doesn't handle panics, but there is a configurable `Recover` middleware which does so. I agree though it's unusual, and this stuck out to me as an exception that proves the rule.

[0] https://echo.labstack.com/middleware/recover/

I believe the idea is that 'panic' is considered something fatal. If it happens, the application should die unless you have a strong reason for it not to. If you can recover e.g. by returning HTTP 500 for a single request, it should be handled by returning errors up throughout the call stack, i.e., by error handling instead of panicking.

It's definitely an opinionated approach though.

> I believe the idea is that 'panic' is considered something fatal.

I think opinions in 3rd party Go code on what “fatal” means vary, that’s the issue. Sure it was intended to mean “this error is so bad the entire program needs to die right now” but in practice there’s cases where it’s treated more like an unchecked exception in Java, i.e., “I can’t recover from this so _I’m_ going to give up but _you_ can keep going.” Or put another way, what a library might consider fatal the caller doesn’t. It can be argued whether or not that’s the right thing to do, but the fact is it happens in the wild, so for something like a server you probably should trap it.

> but in practice there’s cases where it’s treated more like an unchecked exception in Java, i.e., “I can’t recover from this so _I’m_ going to give up but _you_ can keep going.”

Java has a supertype for unrecoverable problems like out-of-memory-errors. It's called "Error", a subtype from Throwable. Exceptions are also Throwables, but they are NOT errors.

There's nothing unrecoverable about Error exceptions. If one thread hit a bug and threw StackOverflowError, it's not a reason to kill the entire application or even thread itself, just unwind stack, show some error and continue processing next task. The only tricky situation I'm aware of is OutOfMemoryError, because it can affect other threads. I'd prefer to restart the server. But again it's just because typical code allocated heap all the time. One can write code carefully without heap allocations and this code will work just fine with OOM errors thrown around.

> It can be argued whether or not that’s the right thing to do

No, because both approaches could be right at a higher level, to pretake that decision at this level is definitely wrong here?!

Nil pointer panics happen. When a DB column that was supposed to be non-null is null and the code unexpectedly accesses it. Oh schema won't help because legacy. There are rare though and we fix such issues as they are discovered, but just pointing out a benign case when crashing the whole server on panic may be a wrong approach. Our approach is to always recover from panics so the server keeps chugging, but raise an alarm so someone is paged and fixes the issue right away.

The question: what is the state of your server after a handler panics? The answer: you have no idea. It is not wise to continue serving requests when you may have serious issues in the state of your server. Maybe some central data structure is now corrupt. You have no way of knowing. Fail fast and fail hard.

OTOH, maybe your priorities are different and you would prefer to be more available than correct. In that case by all means add a recover to the top level of your request handlers. But it was a mistake to have made this decision for all users of net/http ahead of time.

In my experience, the panic is most likely because someone accessed a nil field when adapting some data. Nothing is corrupt, we just threw an exception in a mundane way.

The reality is this is far more common than something truly fatal. Mistake for someone or not, it probably is correct for most use cases

Yes, why they made that decision back then. However in the end the lib cannot and shouldn't know, and this decision needs to be intentionally done by the middleware. Correct for most use cases is not good enough here, should be always correct for such fundamental thing.

But that implies your request checking is off; if your server expects a field to be filled in and a client doesn't send it, that should be a HTTP 400 Bad Request error, not an internal server error.

C/C++ based servers running into an error like that would possibly be open for attacks. Go will be a bit more resilient, but it's still better to avoid situations like that.

That said, logging the error and going on serving responses is fine I think (pragmatic), as long as the error is analyzed. But an error that doesn't trigger immediate action is a warning, and warnings are noise [0].

[0] https://dave.cheney.net/2015/11/05/lets-talk-about-logging#:...

You're assuming that the code was written exception safely. E.g.

    mu.Lock()
    foo := bar[baz]    // <- throws exception / panics
    mu.Unlock()

Go is sold as a language without exceptions, so people don't write exception-safe code. Which is fine, except when exceptions are actually caught.

That wouldn't pass a code review where I work... Use a defer to do the unlock

Does defer get called on panic? I thought panic cancels all further execution

Yes it does, which is why recovering from a panic can be done in a deferred function. The go runtime maintains enough metadata to track what deferred functions need to be run while unwinding the stack.

I would also not allow it. I'm saying the problem is that core Go developers say "Go doesn't have exceptions", which is manifestly false, but causes people to not write exception safe code.

But despite you and me, I'm saying there's a lot of broken code out there because of this doesn't-but-actually-does misinformation.

And it's very annoying that you have to tell people to do:

    var i int
    func() {
      mu.Lock()
      defer mu.Unlock()
      i = foo[bar]
    }()

Clean code, that is not. (even if you simplify it by having the lambda return the int)

This is like the biggest thing scaring me away from Go. This half-assed "we don't use exceptions, so you shouldn't have to care about it, except when we do, so you still must write proper defers, which are now doubly verbose because nobody considered it a primary use case"... In any other language, a mutex outside a using/with/try-with/RAII would be instantly flagged in code-review or linter tools. In many cases even hard to write incorrectly, due to entering context being only way to acquire the lock.

Now this middle ground leaves you having to write triple verbose if err != null on every third line of your code and still not be safe from panics-that-shouldnt-have-been-panics.

As parent says, the only way panics can ever work is if the top-level never catches and recovers from them. I'm no expert in go but that would mean in such perfect world, defer should hardly ever be needed at all, not even for locks? Only for truly external resources? But now with popular web servers doing such recovery, the entire ecosystem got polluted and all need to handle it?

The main issues that usually arise as a result of catching panics in handlers like HTTP is that unless code it written very deliberately to be able to recover from being interrupted in the middle of any function call, there is a high risk of e.g. mutexes left locked, which in turn leads to a (silent) program deadlock in general.

This has happened during my couple years at Google at least once, even though it wasn't in an HTTP handler, but the issue was very similar.

> most likely

> probably is correct

Yeah I don't know...

> In my experience, the panic is most likely because someone accessed a nil field when adapting some data. Nothing is corrupt, we just threw an exception in a mundane way.

Even more so if a database is involved (which is generally the case), because odds are the transaction just gets rolled back and there's basically nothing that could be corrupted.

How I understand it is that it's up to the user to create a panic handler middleware, which is perfectly valid and what most people are doing anyway.

Something like: https://github.com/go-chi/chi/blob/master/middleware/recover...

Presumably you have a load balancer in front of your Go code that's written using saner languages and assumptions that does handle errors correctly.

No, that's about error handling: panic/recover in the standard net/http server is used in lieu of exception handling, which is absent in Go. Instead, they (and official Go docs as well) recommend to use "if-hell".

I've always thought that the ideal is somewhere in between the two.

1. Catch the panic/exception.

2. Track the rate of these panics or exceptions. If it is too high some data structure has probably been corrupted or some lock has been poisoned. If a lot of requests are failing abort.

And ideally: 3 signal that you are in a degraded state so that some external process can gracefully drain your traffic and restart you. Although very few people have this level of self-healing infrastructure set up.

I wrote a web server that handled a lot of requests, and my solution to 2. was to have it notify me via our alert chat channel every time it panicked. This was rare enough that I could investigate them individually, and if it got overwhelming (it never did) I could choose to do something fancier.

> Is this another case of Google forgetting that people use Go outside of Google, or am I reading too much into this?

Whether or not they are forgetting aside, this is Google’s style guide for code bases in Google. I don’t think non-Google Go programmers were a consideration for them.

On a broader note, it seems as though anytime Google publishes something people interpret it as “industry standard” (see their C++ style guide) and apply it to their non-Google projects. I personally don’t see this as healthy.

I think on average google stopped being a technical leader years ago. They produce a fair amount of technology, but they also employ nearly 1% of the software engineers in the country. That's a long way to say I would gladly wager that there are dozens of people who read this thread that could produce by themselves and in one session a document which would serve as good or probably better than this.

Reply on news.ycombinator.com