14 ms·

Go Style

• bushbaba 4y ago
  Isn’t the point of go is that go fmt follows the languages global style guide?
  • marcyb5st 4y ago
    It includes things that are not formatting specific like naming (https://google.github.io/styleguide/go/best-practices https://google.github.io/styleguide/go/best-practices)
  • roessland 4y ago
    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.
  • ainar-g 4y ago
    Formatting ⊊ Style Style may also include naming, documentation requirements, recommendations regarding function length, etc.
  • sethammons 4y ago
    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.
    • Beltalowda 4y ago
      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.
      • metaltyphoon 4y ago
        I hate this about default rustfmt.
  • rob74 4y ago
    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...
  • shp0ngle 4y ago
    That is just a formatting guide. How to write spaces, newlines, semicolons. Go fmt doesn't change anything that is not formatting.
    • hoosieree 4y ago
      Yet.
  • liampulles 4y ago
    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.
• cytzol 4y ago
  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?
  • marcus_holmes 4y ago
    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.
  • andrew_eu 4y ago
    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/ https://echo.labstack.com/middleware/recover/
  • evercast 4y ago
• marcus_holmes 4y ago
  > 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.
  • xiaq 4y ago
    > 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.)
    • tragomaskhalos 4y ago
      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
      • deleted 4y ago
• frou_dh 4y ago
  Kinda amusing how the following section is complicated by the language's casing-based visibility feature https://google.github.io/styleguide/go/decisions#initialisms https://google.github.io/styleguide/go/decisions#initialisms
  • watt 4y ago
    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?
    • Joker_vD 4y ago
      Because "ID" is short for "Identification Datum".
      • benji-york 4y ago
        citation needed
    • icedchai 4y ago
      It is commonly abbreviated that way in normal life (see https://en.wikipedia.org/wiki/Identity_document https://en.wikipedia.org/wiki/Identity_document ) When I see "Id" in code, I wince.
      • euroderf 4y ago
        Rage. Raging Id.
    • benji-york 4y ago
      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
• whalesalad 4y ago
  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
  • is_taken 4y ago
    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 }
    • Cthulhu_ 4y ago
      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.
      • whalesalad 4y ago
        I’m used to parsing a date time with things like %Y-%m-%d not the asinine way Go does it.
        • abdusco 4y ago
          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 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.
    • twblalock 4y ago
      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.
• encryptluks2 4y ago
  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.
  • ErikCorry 4y ago
    They could call it "Google Code" /s
  • jefftk 4y ago
    https://gerrit.googlesource.com/ https://gerrit.googlesource.com/
    • jeffbee 4y ago
      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/ https://google-styleguide.googlecode.com/ and now that's a 404.
  • metaltyphoon 4y ago
    Less free? Microsoft didn't change a damn thing on GH, in fact its only getting better after acquisition.
    • jen20 4y ago
      Another case of the lack of distinction in English between "libre" and "gratis" causing problems, I see.
• adamgordonbell 4y ago
  > 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.
  • don_albrecht 4y ago
    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.
    • thunky 4y ago
      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.
      • philote 4y ago
        I agree. Plus, with IDEs and auto-completion, why not use longer names?
        • vbezhenar 4y ago
          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.
        • GuB-42 4y ago
          - 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.
• ltbarcly3 4y ago
  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.
• gopher_1234 4y ago
  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.
  • rand0m4r 4y ago
    Totally agree, panic in a library is a smell
  • jrockway 4y ago
    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.
  • selfmodruntime 4y ago
    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.
• sidlls 4y ago
  > 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.
  • thomascgalvin 4y ago
    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)`.
    • deleted 4y ago
      [deleted]
    • everybodyknows 4y ago
      Consider variadic arguments for such cases.
      • sidlls 4y ago
        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).
    • sidlls 4y ago
      No doubt: and that is generally the pattern I’ve seen. It’s really an eyesore.
    • euroderf 4y ago
      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.
• oracardo 4y ago
  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
  • nasretdinov 4y ago
    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.
  • marwatk 4y ago
    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.
    • Beltalowda 4y ago
      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.
• mroche 4y ago
  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.
  • CSSer 4y ago
    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.
    • kevmo314 4y ago
      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.
  • gilgad13 4y ago
• throwaway2203 4y ago
  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.
  • Thaxll 4y ago
    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.
    • throwaway2203 4y ago
      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?
      • Thaxll 4y ago
        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.
        • notpushkin 4y ago
          Because I want to know what function does?
• keybored 4y ago
  > 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.
• leetrout 4y ago
  > 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.
  • robmccoll 4y ago
    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.
    • masklinn 4y ago
      > 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?
      • jrockway 4y ago
        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.
• BrainVirus 4y ago
  >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?"
  • B-Con 4y ago
    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.
  • Ferret7446 4y ago
    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?
    • michaelcampbell 4y ago
      As does most code. That doesn't mean it has a high signal:noise ratio.
      • robertlagrant 4y ago
        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.
• everybodyknows 4y ago
  > 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.
• matttproud 4y ago
  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.
• mseepgood 4y ago
  > Go tips - stay tuned I'm looking forward to this document.
• panabee 4y ago
  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 https://github.com/HotpotDesign/Developer-Style-Guides could anyone kindly recommend better ones?
  • akshayshah 4y ago
    https://github.com/uber-go/guide/blob/master/style.md https://github.com/uber-go/guide/blob/master/style.md
• gregwebs 4y ago
  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.
• kristofarkas 4y ago
  Somewhat off topic, but what template does this style guide use. Very clean, want to use it myself. Any similar ones also appreciated!