StevenMaude/practical-go.md

## practical-go.md

      
    Raw
  

              practical-go.md
            
          
    Notes from reading Practical Go

Notes of the key points copied/summarised from Dave Cheney's Practical
Go
for more concise reading/reference.
It's worth reading the original document as there is more exposition
there of the ideas presented.
These are terse notes only; I'm not providing my own commentary, opinion
or judgement on the suggestions here.
Variable naming


Longer names can indicate more importance.
Longer names may be more useful where a name is used further from its
definition (e.g. far apart in same source file; in a different
package)
Don't name for types.

If the type describes the thing well, then short variable name may
be appropriate


May want to match names to existing convention elsewhere.
Be consistent; use same parameter name for functions/methods.

Declaration


Be consistent in how you declare variables.
Use var for declaring, but not explicitly initialising a variable.

var then indicates variable has been left at zero value
deliberately.
Consistent with declarations at package level


Use := when declaring and explicitly initialising.
May except rules for consistency, e.g. when you have two related
variables where one has a zero value and one does not (e.g. min,
max)
May also except rules and use var to indicate something more
complicated is happening.
But follow local style of the project.

Comments

Document either "what", "how" or "why".


"What", explain what things available publicly do.
"How", how something works, e.g. commenting inside a function.
"Why", explain why something is the way it is, external factors for
context.

Other points


Comments on variables and constants should describe contents, not
purpose.
Variables and constants without initial values should describe what is
responsible for initialising it.
Document public symbols. There may be exceptions for public methods
that implement an interface.
Raise issues over using comments as TODOs.
Refactor code over commenting code; e.g. maybe a piece of code should
be a separate function.

Packages

Naming and organisation


Changes to packages should be isolated from the rest of a codebase.
Name packages for services they provide, not what they contain.
Package names should be unique. If two packages need the same name:

Then the name may be too generic.
The packages overlap in functionality; merge the packages or review
the design.


Avoid package names like common, util, helpers.

"utility packages"
They contain unrelated functions. Difficult to describe what the
package provides. So, named for what it contains.
Often appear where larger projects have deep package hierarchies and
want to share code without import loops.
Analyse where they are called and move the relevant code into the
caller's package. May be better to duplicate code than introduce an
import dependency.
If you do use utility packages, prefer multiple packages, each
focused on one aspect over a monolithic package.
Example: base, common used where functionality for multiple
implementations, or common types for client and server are in a
single package. May be preferable to put client, server and common
code in a single package, e.g. net/http does not have client and
server subpackages; it has separate files client.go, server.go
for respective types and transport.go for common transport code.


Note that package name is included in a qualified identifier when
called by other packages, e.g. Get() in net/http is http.Get().

Return early


In Go, the style is that the successful path is down the screen as the
function progresses.
Use guard clauses, conditional blocks which check conditions when
entering a function, and return errors early. Makes it clear to the
reader when the program flow gets to the error.

Make zero values useful


nil slices are still useful, you can append to them and don't need to
make them, you can just declare them.
Example: bytes.Buffer can be written to it without initialisation.
nil pointers can still have methods called on them; those methods can
check if the value is nil and provide suitable default values.

Avoid package level state


Programs should be loosely coupled; changes in one package should have
a low chance of affecting another package that does not directly
depend on the first.
Loose coupling in Go:

Use interfaces to describe behaviour of functions or methods.
Avoid global state.


Mutable global state introduces tight coupling; global variables are
invisible parameter to every function in your program.
Any function that relies on a global variable can be broken if:

the variable's type changes;
if another of the program changes the variable.


Favour moving these variables to fields on structs.

Project structure


Projects should have a clear, single purpose.
For applications, you may have one or more main packages inside a
project.
Only have public (exported) or private (not exported) access for
identifiers.
Consider fewer, larger packages: more packages means more types being
public.
Avoid elaborate package hierarchies; they don't have meaning in Go.
You shouldn't have intermediate directories with no .go files.

Arrange code into files

Prefer internal to external tests


Internal tests, e.g. you have a package http2, then you can write
http2_test.go and use package http2 inside that test file.
External tests are where you have a package identifier ending in
test, e.g. package http2_test. These test files can be stored in
the same place as the http2 package, but they are not part of the
same package. These tests are written as if you were writing a
separate package calling your code.
Example test functions for use in godoc should be in an external
test file. This ensures that when viewed in godoc, the examples can
work by copy-pasting.

Use internal packages to reduce public API surface


For projects with multiple packages, you may have exported functions
for use by other packages in the same project, but not part of the
public API.
Use internal in these cases: Go checks that the package that is
doing the import is within the tree rooted at the parent of the
internal directory, e.g. a package …/a/b/c/internal/d/e/f can only
be imported by code in the directory tree rooted at …/a/b/c.

Keep main package small


Often assume that the things called in main.main() will only be
called in that function, and only called once. This can make it hard
to test.
main should parse flags, open connections to databases and loggers
etc., then hand off execution to some high level object.

API Design

Design APIs that are hard to misuse


Be careful with functions with multiple parameters of the same type:
some don't matter which order parameters are entered (e.g. func Max(a, b int) int) but some do func CopyFile(to, from string) error.

Consider making a helper type that calls CopyFile, then use as
from.CopyTo(to); CopyFile could be made private.


Design APIs for their default use case


Your API should not require the caller to provide parameters which
they don't care about.
Discourage use of nil as a parameter, or at least don't mix nil
and non nilable parameters in the same function signature.

The use of nil can pervade through functions, if one calls
another.
API user may infer that all parameters can be nil.


Prefer variadic over []T parameters

May have functions like func ShutdownVMs(ids []string) error that
often require single arguments to be placed in a slice
Also means you can pass empty slice or nil and it will compile, so
you should cover these cases in testing.
Instead you can use variadic parameters: but may want to have a
single first parameter and a rest ... variadic parameter to
ensure your function isn't called with no arguments.


Functions should define the behaviour they require


Idea of "interface segregation".
As an example, consider func Save(f *os.File, doc *Document) error
Means Save() is tied to real files, can't save to network (would
require changes), can't easily test (real files), also *os.File has
lots of methods irrelevant to Save().
Better to describe the parts of *os.File that are relevant: func Save(rwc io.ReadWriteCloser, doc *Document) error
Takes an interface that describes more general file-shaped things;
broader in application and clarifies which methods in *os.File are
relevant to the function (and hides irrelevant methods).
Can reduce to func Save(wc io.WriteCloser, doc *Document) error as
unlikely that this function should read the file, if it's purpose is
to save only.
But being able to close the file has two issues: it raises the
question of when the wc will be closed, and it may be that the
caller might want to add further data after saving this document.
Now this function is specific in terms of requirements — it needs
something writable — and general in terms of function, anything that
implements io.Writer interface.

Error handling

Eliminate error handling by eliminating errors


Example: using a loop controlled by bufio.NewScanner(r io.Reader)
with for sc.Scan() to count lines. This returns true if the
scanner matches a line of text and has encountered an error. It avoids
manually checking errors in the loop to break out of the loop. It
also handles io.EOF by converting it to nil unless another error
is encountered; other approaches might see you have to manually check
whether the error is io.EOF or not.


Example: replace an io.Writer with an errWriter that's a struct
with an io.Writer and an error:
type errWriter struct {
  io.Writer
  err error
}

func (e *errWriter) Write(buf []byte) (int, error) {
  if e.err != nil {
  	return 0, e.err
  }
  var n int
  n, e.err = e.Writer.Write(buf)
  return n, nil
}

func WriteResponse(w io.Writer, st Status, headers []Header, body io.Reader) error {
  ew := &errWriter{Writer: w}
  fmt.Fprintf(ew, "HTTP/1.1 %d %s\r\n", st.Code, st.Reason)

  for _, h := range headers {
  	fmt.Fprintf(ew, "%s: %s\r\n", h.Key, h.Value)
  }

  fmt.Fprint(ew, "\r\n")
  io.Copy(ew, body)
  return ew.err
}
Now, the errors that would have to be checked after each fmt.Fprintf
don't need to be; errors are stored on the errWriter, if there's
already an error, no further write is attempted, and the error is
returned at the end. It also enables just returning the error from
errWriter, not having to pull out the error from io.Copy's return
values.


Only handle an error once


Should only make one decision in response to a single error.
Example: logging an error, returning to caller that might also log it
and return it, back to the top of the program. Can end up with
duplicate log lines and little context at the top of the program for
the error.

Adding context to errors


Ensure that you do return after an error. This may cause bugs if you
don't: subsequent steps may still "succeed" without error, and return
without an error even though the behaviour is not correct.
Can use fmt.Errorf to add context to an error.
Example: return fmt.Errorf("could not write config: %v", err)
This removes the possibility of logging an error, but forgetting to
return it.

Wrapping errors


fmt.Errorf works for annotating the error message, but obscures the
original error type. This shouldn't matter if all you are doing is
checking it is not nil and then logging/printing it.
There may be less common cases where you want to recover the original
error.
It mentions github.com/pkg/errors for this purpose though wrapping
errors is supported in Go directly
now.

Concurrency


Goroutines are often overused.
If main.main() of a Go program returns, then the program exits
regardless of any other goroutines started by the program.
Example: calling http.ListenAndServe() in a goroutine called from
main() compared with calling the function directly in main(). If
the call is in a goroutine, you then have to stop main() from
exiting.
If a goroutine can't progress until it gets the result from another,
sometimes it is easier to not delegate the work.

Leave concurrency to the caller


If your function starts a goroutine, provide the caller with a way to
explicitly stop that goroutine. It is often easier to leave the
decision to execute functions asynchronously to the caller.
Example:
// ListDirectory returns a channel over which
// directory entries will be published. When the list
// of entries is exhausted, the channel will be closed.
func ListDirectory(dir string) chan string

The alternative might be to put all the entries into a []string and
return []string, error, but could take a long time and allocate lots
of memory.
This implementation is likely using a goroutine.

Note: channels don't require goroutines to use, but they most likely
do involve use of goroutines (alternative is to create a channel
large enough for all the values being stored, then close the channel
and return it to a caller).


Problems:

Using a closed channel to indicate there are no more entries means
that the caller cannot tell if an error occurred. It is not possible
for the caller to distinguish an empty directory and a failure to
read the directory.
The caller has to continue to read from the channel until it is
closed to know that the goroutine filling the channel has stopped.
The caller might have received the answer they wanted before that.
It might be more memory efficient than the slice implementation,
especially for larger directories, but it is no quicker.


Solution: pass a function to ListDirectory that is called for each
directory entry:
func ListDirectory(dir string, fn func(string))


Never start a goroutine without knowing when it will stop


Applications should be restarted from outside the application, not
responsible for restarting themselves.
Only use log.Fatal() from main.main or init functions.

If used inside functions, the program will just stop. This makes
them difficult to test.


If using goroutines, then a good approach is passing errors back to
the originator of the goroutine and letting the caller shut down the
process cleanly.
The example given involved running http.ListenAndServe() in
goroutines for application and debugging separately.
Code:
func serve(addr string, handler http.Handler, stop <-chan struct{}) error {
	s := http.Server{
		Addr:    addr,
		Handler: handler,
	}

	go func() {
		<-stop // wait for stop signal
		s.Shutdown(context.Background())
	}()

	return s.ListenAndServe()
}

func serveApp(stop <-chan struct{}) error {
	mux := http.NewServeMux()
	mux.HandleFunc("/", func(resp http.ResponseWriter, req *http.Request) {
		fmt.Fprintln(resp, "Hello, QCon!")
	})
	return serve("0.0.0.0:8080", mux, stop)
}

func serveDebug(stop <-chan struct{}) error {
	return serve("127.0.0.1:8001", http.DefaultServeMux, stop)
}

func main() {
	done := make(chan error, 2)
	stop := make(chan struct{})
	go func() {
		done <- serveDebug(stop)
	}()
	go func() {
		done <- serveApp(stop)
	}()

	var stopped bool
	for i := 0; i < cap(done); i++ {
		if err := <-done; err != nil {
			fmt.Println("error: %v", err)
		}
		if !stopped {
			stopped = true
			close(stop)
		}
	}
}

This approach has the http.ListenAndServe() calls return errors via
a channel to main.main() instead of immediately calling
log.Fatal().
Then in main.main(), read from this done channel reporting errors
and then close a stop channel which goroutines that the servers
start are receiving from. This then fills the done channel that is
being used to take the return value from the goroutines; the capacity
of this done channel is checked in a loop in main.main(): when
filled, the program exits.