Idiomatic Go (2016)
dmitri.shuralyov.com
dmitri.shuralyov.com
> Avoid unused method receiver names
That's just "don't create a non-static method if you don't use 'this'" in other languages with classes. Which is not a bad idea, but hardly matters. If someone realizes that it's difficult to add a unit test for the function, they'll fix it themselves. Maybe the real question is: did the code author create a unit test for the function?
(P.S. I never understood why Go uses words like "receiver" and "marshaling" that are rarely used elsewhere)
> Empty string check
Sure, but that's just ordinary code smell, likely due to someone not thinking carefully over the code, which could be easily identified in a code review. If it's never found, it barely matters anyway. Is it really worth bringing this up?
Everything else seems unnecessarily boring, like the number of spaces between sentences. Most people I know write comments like how they write regular English sentences, like in emails. Other than a professor who is near retirement age, I don't know anyone who uses double spaces. It's so rare I don't see any point in mentioning it. And if someone did do this in the code, whether it's their style or by accident, I wouldn't even flag it in the code review. For what? Does anyone benefit from it? It's important to have consistency over fundamental things like tab vs space, but that quickly becomes meaningless bikeshedding.
My advice would be: write code using your common sense. Don't be obsessed with trivial things that don't matter. Use your time elsewhere -- push a new feature, fix a bug, add a unit test, or just relax.
> That's just "don't create a non-static method if you don't use 'this'" in other languages with classes.
I don't think that's what's meant. It means to write `func (Receiver) Method()` rather than `func (r Receiver) Method()` when you don't use `r`. Sometimes you need this to implement an interface like `error` or `Stringer` and you just don't need instance data.
(I'm no Go apologist but I think "receiver" is a great term. It's clear, it's applies to other languages and paradigms, and it doesn't really have an alternative that I'm aware of.)
Go doesn't have static methods. Regardless if you name the receiver, you have to provide an instance of the type to all methods. Maybe you should check your own knowledge before criticizing others
Not sure about receiver, but marshaling has been part of Python's lingo for ages.
So Alan and the language he helped create. That’s double counting the sample of 1
What else are you familiar with? Those are extremely common terms in the object oriented community.
I'm with you regarding marshaling, but not because it's not an industry-standard term, it's just that Go misapplies it. Marshaling historically has referred to describing a remote procedure call and its return value; for example, you call "CreateFoo()" with a "CreateRequest", the latter must be marshaled into a payload that the remote call can read. For a network call this requires serializing the data to bytes using some kind of format, but for a local call it could just be a pointer to a data structure. However, historically people have often mixed the two terms. Python's standard library also misuses "marshal" to refer to serialization.
If you’re focused on what character of “GitHub” or “oauth” should capitalised in a variable name, then you really are focusing on the wrong problems of software development.
The more uniform a code base, the easier it is to breeze around and get stuff done. Naming things is already hard, so having rules around the annoying bits is nice.
If the author really cares this much then they should be linter rules instead of a blog post.
This way people won't start bikeshedding about the placement of curly braces, they either accept the style or move on.
It also prevents things from getting political or personified. It's the tool that's making the decisions, not a specific person.
Just write a linter rule and be done already.
```suggestion
my_change
```
if there's someone on your team prone to style nitpicks like this, this can often sate them, and it's convenient for you to merge into your branch
This article reads as a parody of the worst sort of code review. This seems to be a person who sees themselves as little more than a regex engine.
Were I writing a go codebase in the UK, all spellings would be UK -- because of how absurd it would be to retrain the staff to split their brain on a trivial issue of this kind.
Likewise plurals do not matter, -- double spacing in comment sentences? I cannot imagine comment whitespacing being on the radar of any person one would wish to have as a team mate.
What about if you were in Australia? Germany? Poland? Bulgaria? China?
> how absurd it would be to retrain the staff to split their brain on a trivial issue of this kind.
I've seen this argument used to argue that all the spellings in source code should be US English, always and everywhere. In my opinion, this (being able to use the same argument to argue both for and against the same issue) invalidates the argument entirely.
Do you apply this rule uniformly, or just to spellings?
The level of time-wasting here is off the charts. Write code to solve problems. If you have an international team, have international standards, if you do not -- it does not matter.
If any of this changes, literally, apply a regex. OP is a blog article about rules for a code review -- as if the relevant part of the review is spelling ? Are we mad?
Code review isnt to enforce these superficial standards -- these are regexes/flags in a build process. Code review is to ensure the code solves the problem under functional/non-functional constraints, in a manner which is easy to understand, communicates intention, etc.
Anyone talking about whether something is pluralised in a code review is a person who should be no where near any such process. They are clearly pathologically incapable of prioritisation or completely uninterested in review.
The link (https://go.dev/talks/2014/names.slide#14) says:
Error values should be of the form ErrFoo:
var ErrFormat = errors.New("image: unknown format")
// But if you want to give it a longer name, use "somethingError".
var specificError error
result, specificError = doSpecificThing()
Don't do this:
[...]
var errSpecific error
result, errSpecific = doSpecificThing()
The article, on the other hand, advocates for local variables storing errors not to have distinct names.
And I'm only asking about when you are giving an error a distinct name, not just naming it 'err'.
“oAuth is not pretty” but “oauth” is ?
We’ve all had PRs reviewed by people like this and we know where it leads.
Agreed. Functions shouldn’t be full of short non-descriptly named variables.
The longer the lifetime/scope of a variable, and the more variables there are, the more descriptive the names should be.
for i := range personlist {
person := personlist[i]
...
}
for personNum := range personlist {
person := personlist[personNum]
...
}
I say this as someone who is simultaneously impressed by some parts of it, and gobsmacked at other parts.
Just ignore the dogmatic Gopher priests.
I'm not aware of any other language that does this. It's common for other languages to use underscores to indicate that something should be private, but I can't think of another example that uses either capitalization for access control or that embeds syntactic meaning in variable names. To me that makes Go unusual, and for a language that cares greatly about being accessible, unusual choices like this seem silly - not bad or foolish, necessarily - just silly.
I think most people have to be told about the capitalization rule (by the compiler, docs, etc.) because the rule itself isn't that obvious to someone coming from another language, that was my experience at least, and in that sense it feels a bit like a secret handshake to me.
I might agree with you if the UC/LC innovation were burdensome to "implement" (i.e. to remember). But, well, it's about as burdensome as understanding that on Unix, a filename that starts with a dot is normally hidden. In other words: instantly grokable and memorable.
> once you know what the rule is I agree that it makes access control obvious
The rule is memorable and grokable, I agree with you! I don't use "silly" to characterize it as the wrong choice, I strictly mean it's a quizzical choice.
Using your hidden file analogy in Unix: if I were to invent a new file system and I decided to use lower case letters to represent hidden files instead of borrowing the "." convention, wouldn't that strike you as a silly choice given the ubiquity of Unix-like filesystems?
But sure, I'll sign on to "quizzical". Mainly I'm glad that in Go there's only two levels of visibility, not 3+. Much easier to reason about. Altho I'll admit to being confused when you mix the two levels in the names of a struct and its fields.
The only reason I'd ever consider telling someone that "canceled" is wrong is if the other spelling was firmly established in the actual codebase. Not in comments. And absolutely not with the ridiculous claim that the language you're coding in has opinions about how to spell your comments.
https://books.google.com/books/about/Learning_Go.html?id=vjA...
I’m confused by the examples in this section. Are they not identical?