Go Code Review recommendations

This is a creation in Article, where the information may have evolved or changed.

Note: The original text is from Go-wiki to go Code Review Comments

Go Code Review recommendations

This page collects common comments from the Go code review so that a detailed description can be quickly referenced. This is a common error list, not a style guide.

You can watch effective go as a supplement.

Please discuss this change before editing this page , even if it is a small change, many people have their own ideas, this is not a battlefield.

Gofmt

Run gofmt to automate the problem of solving your code's main mechanical style, with almost all the informal go codes using GOFMT. The remainder of the document deals with non-mechanical style points.

Another alternative is to use Goimports, which is the parent set of the GOFMT, adding additional (and removed) import rows.

Comment sentence

View Http://golang.org/doc/effective_go.html#commentary. The note document declares that it should be an entire sentence, even if it seems a bit redundant. This method makes it very formatted when it is extracted into the Godoc document. The comment should begin with the name of the thing being described and end in a period of time.

// A Request represents a request to run a command.type Request struct { ...// Encode writes the JSON encoding of req to w.func Encode(w io.Writer, req *Request) { ...

Wait a minute.

DOC recommendations

All top-level, exported names should have a document comment as Non-trivial unexported type and feature declaration. See Http://golang.org/doc/effective_go.html#commentary For more information on the annotation conventions.

Do not use Panic

See http://golang.org/doc/effective_go.html#errors, do not use panic for normal error handling. Use error and multiple return values.

Error Strings

The Error strings should not be capitalized (unless a proper noun or initials is narrowed) or ended with a punctuation mark. They are usually printed in other contexts. Even with FMT. Errorf ("something Bad") instead of FMT. Errorf ("Something bad"), so that log. Print ("Reading%s:%v", filename, err) format does not have a false uppercase letter intermediate message. This does not apply to log records, which are absolutely line-oriented and are not included in other information.

Note: The above Errorf difference is inside the content s This letter one uppercase one lowercase "

Handling Errors

Look at Http://golang.org/doc/effective_go.html#errors, do not use _ variable discard error, if a function returns an error, check it to ensure that the function is successful. Handle this error, return it, or use panic in the case of a real exception.

Imports

Imports are organized as groups, with blank lines between them, and standard packages in the first group.

package mainimport (    "fmt"    "hash/adler32"    "os"    "appengine/user"    "appengine/foo"    "code.google.com/p/x/y"    "github.com/foo/bar")

Goimports will help you do this.

The point number of the Import

The point number form of import is very useful for testing and cannot be part of the package being tested due to cyclic dependencies.

package foo_testimport (    . "foo"    "bar/testutil"  // also imports "foo")

In this example, the test file cannot be in the Foo package because it uses bar/testutil and it also introduces Foo, so we use the ' import. ' form to disguise it as part of the Foo package, even if it is not. In addition to this situation, do not use ' import ' in your program. It will make your program difficult to read because it is difficult to understand, a name such as Quux whether the current package is a top-level identifier or in an ingest package.

Indent Error Flow

Try to save the normal code path in one of the smallest code indents, and indent error handling, first handling the error. This improves the readability of the program, which is in line with the fast scanning habits of vision. For example, do not write these:

if err != nil {    // error handling} else {    // normal code}

And it should be written like this:

if err != nil {    // error handling    return // or continue, etc.}// normal code

If the IF statement has an initialized statement, like this:

if x, err := f(); err != nil {  // error handling  return} else {  // use x}

This requires that a short variable declaration be moved to its own line.

x, err := f()if err != nil {  // error handling  return}// use x

Initial letter Abbreviation

The name of the word should be initials (for example, "URL" or "NATO") is the same case. For example, "url" should be displayed as "url" or "url" (like "urlpony", or "urlpony"), not a URL. Here's an example: Servehttp instead of servehttp.

This rule also applies to "ID" when it is "identifier" short name. Therefore, use "AppID" instead of "AppID".

The code generated by the protocol buffer compiler is out of this rule. Humans write code that should be more high-standard than machine-written code.

Line length

This does not have strict restrictions in the Go code, but to avoid too long lines, again, when they are at a more readable length, they should not add line breaks to make them shorter---for example, if they are duplicates.

The recommendation is usually no more than 80 characters before wrapping, not because it is a rule, but because it is more readable when viewed in an editor that can display hundreds of columns. Humans are more suited to narrow text relative to a wide text. In any case, Godoc should render it in a better way.

Mixed Caps

Look at Http://golang.org/doc/effective_go.html#mixed-caps, this is even when it breaks the habit of other languages. For example, a non-exported constant is maxLength instead of maxLength or max_length.

Result parameter naming

Consider what this should look like in Godoc, and the resulting parameters are named like:

func (n *Node) Parent1() (node *Node)func (n *Node) Parent2() (node *Node, err error)

The better usage is:

func (n *Node) Parent1() *Nodefunc (n *Node) Parent2() (*Node, error)

In other words, if a function returns two or three parameters of the same type, or if the meaning of a result is unclear through the context, it can be useful to add a name, such as:

func (f *Foo) Location() (float64, float64, error)

Without this clarity:

 // Location returns f's latitude and longitude. // Negative values mean south and west, respectively. func (f *Foo) Location() (lat, long float64, err error)

The Naked return value is good if the function is small. Once it is a medium function, it must be clear your return value, the inevitable result: just so that you use the naked return value is not aware of named result parameters. A clear document is always more important than saving one or two lines in your function.

Finally, in some cases, in order to change it in a deferred closure, you need to name a result parameter. This is usually not a problem.

Naked Returns

View Codereviewcomments#named_result_parameters

Package recommendations

The package recommends that, just as all annotations should be rendered by godoc, they must be adjacent to the packet without a blank line:

// Package math provides basic constants and mathematical functions.package math

/*Package template implements data-driven templates for generating textualoutput such as HTML.....*/package template

View http://golang.org/doc/effective_go.html#commentary For more information about the annotation conventions.

Package Name

The name of all references in your package should use the package name, so you can omit the identifier for that name. For example, if you are in package chubby, you do not need to type Chubbyfile, the client will write chubby. Chubbyfile. Instead, name the type file, and the client will write chubby. File, see http://golang.org/doc/effective_go.html#package-names for more information

Passing values

Do not pass the pointer as a function parameter just to save a few bytes, and if the parameter x referenced by the function is x only, then the parameter should not use the pointer. Common examples include passing a pointer to a string (string), or a pointer to a interface value (*io). Reader). In both cases, its own value is constant and can be passed directly. This advice does not apply to large structs, or to small structs that will grow.

Receiver Names

The name of receiver of a method should be a reflection of its identity, usually one or two letters is sufficient to satisfy its type of abbreviation (for example, "C" or "CL" as "Client" abbreviation). Do not use generic names such as "Me", "this" or "self", and the typical identity for object-oriented languages is to focus more on methods than functions. The name does not have to be described as a method parameter, its role is very obvious and is not intended as a document. It can be very short, as it appears on almost every line of each method; be familiar with the recognition of cleanliness. Need to be consistent, if you call "C" receiver in one method, you cannot invoke "CL" in another method.

Receiver Type

It is very difficult to choose whether to use a value receiver or a pointer receiver on one method, especially for Go novice. If you're not sure, use pointers, but sometimes a value receiver is more meaningful. This is often the result of efficiency, such as small, unchanging structs or the value of the underlying type.
Based on experience, generally speaking:

• If receiver is a map,func or Chan, do not use the pointer
• if receiver is a slice and the method is no longer reslice or redistributed slice, do not use the pointer
• The pointer must be used if the method requires a mutable receiver,receiver
• If receiver is a containing sync. The struct,receiver of a Mutex or similar synchronizing property must be a pointer to avoid copying
• If receiver is a large struct or array, a pointer receiver will be more efficient. How big is big? Suppose it is a parameter to pass all of its elements as a method. If it feels too big, it's also too big for receiver.
• Can function or methods, either concurrently or when called from this method, being mutating the receiver? When the method is called , a copy of a value type is created so that external updates do not affect receiver. If the change in the original receiver must be visible, then receiver must be a pointer.
• If receiver is a struct, array or slice and any one of its elements is a pointer, then it is mutable. Prefer the pointer receiver because it is more explicit to the reader
• if receiver is a small array or struct, it is naturally a value type (for example, time. Time type), with no mutable attributes and no pointers, or just a simple underlying type, such as int or string, a value receiver makes more sense. A value receiver can reduce the amount of memory garbage generated; If a value is passed to a value method, a stack copy will be used instead of allocating memory on the heap (the compiler is trying to intelligently avoid allocating memory, but it may not always succeed). There is no option to select a value receiver for this reason before optimization.
• Finally, if you are unsure, use the pointer receiver

Useful Test Failures

Tests should be accompanied by output-helpful information at the time of failure to tell you what the reason for the failure is, what the input is, what actually happened, and what the expectations are. It is likely to be a helper to write a bunch of assertfoo. But make sure your helper generates useful error messages. Suppose a person who is not you, or someone who is not your team, is debugging your error test. A typical Go failure test should look like this:

        if got != tt.want {                t.Errorf("Foo(%q) = %d; want %d", tt.in, got, tt.want)    // or Fatalf, if test can't test anything more past this point        }

Note that the actual command here should be expected! = and the message also uses this command. Some test frameworks encourage writing these post-write: 0! = x, "expect X to get 0" and so on, Go does not.

If there appears to be many types, you may want to write a Table-driven test: http://code.google.com/p/go-wiki/wiki/TableDrivenTests.

Another common technique is to eliminate the ambiguity of error testing when you use a different input to wrap each call test helper with a different testfoo function, so the failure test uses the name:

     func TestSingleValue(t *testing.T) { testHelper(t, []int{80}) }     func TestNoValues(t *testing.T) { testHelper(t, []int{}) }

In any case, your responsibility is no matter who debugs your code later, when it fails, it should output useful information.

Variable naming

The name of the variable in Go should be short rather than long. This is especially true for local variables with limited space. Prefer C for the number of rows, like I to represent the index of slice.

The basic rule is: further declaration, a name is used, the name must describe more information. For a method receiver, one or two letters are appropriate. Common variables such as loop indices and readers can be single letters (I, R). More unusual things and global variables require a more descriptive name.