x/talks/2014/readability: talk for GoCon 2014 autumn in Tokyo

LGTM=r
R=r
CC=golang-codereviews
https://golang.org/cl/176660043
diff --git a/2014/readability.slide b/2014/readability.slide
new file mode 100644
index 0000000..7cb6645
--- /dev/null
+++ b/2014/readability.slide
@@ -0,0 +1,388 @@
+When in Go, do as Gophers do
+Go Conference 2014 autumn
+30 Nov 2014
+
+Fumitoshi Ukai
+Google Software Engineer - Chrome Infra team
+ukai@google.com
+https://plus.google.com/+FumitoshiUkai
+@fumitoshi_ukai
+
+* Go Readability Approver
+
+A team to review Go readability.
+
+- help to learn idiomatic Go though code review
+- review code of projects that are not main project of the reviewer
+
+- [[http://blogger.ukai.org/2013/12/code-readability.html][I joined the team about a year ago]] as 20% time, and reviewed ~200 CLs
+- For now, I'm reviewing at most 3 CLs per day, 12 CLs per week.
+
+.image readability/project.png
+.caption _Gopher_ by [[http://www.reneefrench.com][Renée French]]
+
+* What is Readability skill?
+
+Literacy of a programming language.
+
+A skill to read or write *idiomatic* code.
+
+Each programming language has its own preferred style.
+In C++, each project chooses a preferred style.
+
+- [[http://google-styleguide.googlecode.com/svn/trunk/cppguide.html][google]]([[http://www.chromium.org/developers/coding-style][chromium]]), [[https://www.webkit.org/coding/coding-style.html][webkit]]([[http://www.chromium.org/blink/coding-style][blink]])
+
+Don't write Go code as you write code in C++/Java/Python.
+Write Go code as Gophers write code.
+
+* Go code should ...
+
+- be articulate, concise.
+- provide a simple API.
+- have precise comments.
+- be readable, top-down code.
+
+_"_Want_to_understand_something_in_google_servers?_Read_the_Go_implementation!_"_
+
+.caption by some Googler
+
+.image readability/pkg.png
+.caption _Gopher_ by [[http://www.reneefrench.com][Renée French]]
+
+
+* Good tools
+
+[[https://golang.org/cmd/gofmt/][go fmt]] - format Go programs.
+[[https://godoc.org/golang.org/x/tools/cmd/vet][go vet]] - report suspicious code
+[[https://github.com/golang/lint][golint]] - report coding style errors.
+[[http://blog.golang.org/godoc-documenting-go-code][godoc]] - browse documenation
+
+# Go code is easy to read for tools too.
+
+* Tools are not enough
+
+# Developer doesn't read code the same as tools.
+# Programs(compiler, etc) will read code if its syntax is ok.
+# Tools are not enough, have false-positive/false-negative.
+# Needs human judgments.
+
+Readable code == easy to recognize, less burden for brain.
+Both writer and reader should have readability skills.
+Go is very simple ([[https://golang.org/ref/spec][lang spec]] is about 50 pages)
+
+.image readability/gophers5th.jpg
+.caption _Gopher_ by [[http://www.reneefrench.com][Renée French]]
+
+* Readability Reviews
+
+- Any mistakes/bugs?
+- Layout?
+- Simple code flow?
+- Simple API?
+
+.image readability/gopher-ok-no.png
+.caption _Gopher_ by [[http://www.reneefrench.com][Renée French]], and [[https://github.com/tenntenn/gopher-stickers/][tenntenn]]
+
+* mistakes/bugs
+
+* error check
+
+original code
+
+.code readability/err_regexp_bad.go
+
+revised
+
+.code readability/err_regexp_good.go
+
+- Check error with [[https://golang.org/pkg/regexp/#MustCompile][regexp.MustCompile]].
+- Must should be used only in [[http://golang.org/ref/spec#Package_initialization][initialization]](package `var` or `init()` ).
+
+- [[http://golang.org/ref/spec#String_literals][Raw string literal]] makes it easy to read regexp.
+
+* error check: original code
+
+.code readability/err_close_write_bad.go
+
+* error check: revised
+
+.code readability/err_close_write_good.go
+
+- Check error of Close for write.
+- No need to use defer, when it's simpler.
+
+* in-band error: original code
+
+.code readability/in-band-error-client.go
+
+.code readability/in-band-error.go
+
+* return value and error: revised
+
+.code readability/val-and-error.go
+
+[[http://golang.org/doc/effective_go.html#multiple-returns][Return error as error, not as some value]]
+
+* error design
+
+If client doesn't need to distinguish errors, e.g. ok with `err` `!=` `nil` check only.
+
+	fmt.Errorf("error in %s", val) or errors.New("error msg")
+
+If client wants to distinguish several errors by error code.
+
+	var (
+	  ErrInternal   = errors.New("foo: inetrnal error")
+	  ErrBadRequest = errors.New("foo: bad request")
+	)
+
+If you want to put detailed information in error.
+
+	type FooError struct { /* fields of error information */ }
+	func (e *FooError) Error() string { return /* error message */ }
+
+	&FooError{ /* set error data */ }
+
+Don't use `panic`.
+
+- But when you do, use it only within the package, and [[http://golang.org/doc/effective_go.html#recover][return error with catching it by recover]].
+
+* nil error
+
+.code readability/nil_error.go
+
+[[https://golang.org/doc/faq#nil_error][FAQ: Why is my nil error value not equal to nil?]]
+
+[[http://blog.golang.org/laws-of-reflection][interface has 2 data]]. interface value is nil == both data is nil.
+
+- value
+- type information
+
+* embed interface: original code
+
+.code readability/implement-interface-bad.go
+
+- `scan.Writer` is an interface.
+- `ColumnWriter` will have methods of the `scan.Writer` interface (i.e. `ColumnWriter` implements the `scan.Writer` interface), but ...
+
+* check interface implementation: revised
+
+.code readability/implement-interface-good.go
+
+- The original author wanted to check `ColumnWriter` [[http://golang.org/doc/effective_go.html#blank_implements][implements]] the `scan.Writer` interface.
+
+* embed interface
+
+If a struct doesn't have a method of a interface explicitly, the interface is embedded in the struct, and you didn't set the interface field to a concrete value (i.e. the interface field value is nil), the method call will panic.
+
+.code readability/nil_interface_en.go
+
+It would be useful in a test when you want to implement only a subset of methods in the huge interface.
+
+* Readable layout
+
+* layout of fields in struct: original code
+
+.code readability/struct-field-bad.go
+
+* layout of fields in struct: revised
+
+.code readability/struct-field-good.go
+
+- Organize fields in groups, with blank lines between them.
+- Put [[https://golang.org/pkg/sync/#Mutex][sync.Mutex]] in top of a block of fields that the mutex protects.
+
+* Long line
+
+.code readability/long-line-fold.go
+
+* into one line
+
+.code readability/long-line-nofold.go
+
+- [[https://golang.org/s/comments#Line_Length][No rigid line length limit]]
+- though, can't we make it shorter?
+
+* Choose concise names
+
+[[https://golang.org/s/comments#Variable_Names][Choose good name in the context]]
+
+- Long names are not always better than short names.
+
+Short and accurate names.
+
+- [[https://golang.org/s/comments#Package_Names][SamplingServer in sampling package is stutter]]. Name `Server`, which clients will write as `sampling.Server`.
+- Use [[https://golang.org/s/comments#Receiver_Names][one or two letters for receiver names]].
+- Use short namse for parameters since type name will give some information.
+- Use descriptive names for basic types, though.
+- Use short names for local variables: prefer `i` to `index`, `r` to `reader`.
+- Short names should be fine if function is not long.
+
+* one line
+
+.code readability/long-line-short.go
+
+* top-down code
+
+* conditional branch
+
+- [[https://golang.org/s/comments#Indent_Error_Flow][Keep the normal code path at a minimal indentation.]]
+
+original code
+
+.code readability/if-else-bad.go
+
+revised
+
+.code readability/if-else-good.go
+
+* conditional branch (2) original code
+
+.code readability/resthandler.go
+
+* conditional branch (2) revised
+
+.code readability/resthandler-fix2.go
+
+- factor out function.
+
+* conditional branch (3): original code
+
+.code readability/if-switch-bad.go
+
+* conditional branch (3): revised
+
+.code readability/if-switch-good.go
+
+- use [[http://golang.org/ref/spec#Switch_statements][switch]]
+
+* Simpler code
+
+* time.Duration
+
+Use [[https://golang.org/pkg/time/#Duration][time.Duration]] ([[https://golang.org/pkg/flag/#Duration][flag.Duration]]) rather than `int` or `float` to represent time duration.
+
+original code
+
+.code readability/time_duration_bad.go
+.code readability/time_duration_bad1.go
+.code readability/time_duration_bad2.go
+
+revised
+
+.code readability/time_duration_good.go
+
+- Don't write unnecessary type conversion.
+- Since [[http://blog.golang.org/constants][const is untyped]], no need to convert 30 to `time.Duration`.
+- Don't write unnecessary comments.
+
+* sync.Mutex and sync.Cond: original code
+
+.code readability/close-cond-bad.go
+
+* chan: revised
+
+.code readability/close-cond-good.go
+
+- You could use [[http://golang.org/ref/spec#Channel_types][chan]], instead of [[https://golang.org/pkg/sync/#Mutex][sync.Mutex]] and [[https://golang.org/pkg/sync/#Cond][sync.Cond]].
+
+* reflect: original code
+
+.code readability/reflect-bad.go
+
+* without reflect: revised
+
+.code readability/reflect-good.go
+
+- Don't use [[https://golang.org/pkg/reflect/][reflect]], if you know the type at compilation time.
+
+* Test
+
+* Test code
+
+.code readability/test-pattern_en.go
+
+- [[https://golang.org/s/comments#Useful_Test_Failures][Have helpful test failure messages]]
+- Don't create yet-another assert function. Use existing programming language features.
+- Write [[https://golang.org/pkg/testing/#hdr-Examples][an example test]] rather than writing how to use API in a doc comment.
+
+.code readability/example_test.go
+
+* Comment
+
+* Comment
+
+[[https://golang.org/s/comments#Package_Comments][Write package comment.]]  Write command comment in main package.
+[[https://golang.org/s/comments#Doc_Comments][Write comments on exported names.]]
+[[https://golang.org/s/comments#Comment_Sentences][Doc comment should be a complete sentence that starts with the name being declared.]]
+
+	// Package math provides basic constants and mathematical functions.
+	package math
+
+	// 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) {
+
+Browse with [[http://blog.golang.org/godoc-documenting-go-code][godoc]]
+
+	$ godoc bytes Buffer
+
+	$ godoc -http=:6060  # http://localhost:6060/pkg
+
+If you feel comments are unclear or hard to write concisely, reconsider your API design.
+
+* API design.
+
+Important to choose a good package name.
+
+- e.g. package `util` is not good name, since most packages are utilities of something.
+# split it into smaller packages, or merge it in bigger package, or wrong package boundary.
+
+Make API simple.
+
+- [[http://golang.org/doc/effective_go.html#multiple-returns][Use multiple returns]]. Don't use pointers as output parameters.
+- Don't use pointer to slice, map, chan or interface.
+- [[http://golang.org/doc/effective_go.html#multiple-returns][Return error as error]]: [[https://golang.org/s/comments#Don't_Panic][don't panic]]
+- Implement common interfaces ([[https://golang.org/pkg/fmt/#Stringer][fmt.Stringer]], [[https://golang.org/pkg/io/#Reader][io.Reader]] and so on) if they match your code.
+- Use interfaces for parameters. They makes it easier to test.
+- e.g.: If a function reads from a file, use [[https://golang.org/pkg/io/#Reader][io.Reader]] as a parameter instead of [[https://golang.org/pkg/os/#File][*os.File]].
+- Prefer synchronous API to async API: refrain from using chan across package boundary.
+- Clients can easily run synchronous API concurrently with goroutine and chan.
+
+* To write readable code
+
+* Code is communication
+
+Be articulate:
+
+- Choose good names.
+- Design simple APIs.
+- Write precise documentation.
+- Don't be too complicated.
+
+.image readability/talks.png
+.caption _Gopher_ by [[http://www.reneefrench.com][Renée French]]
+
+* When you write code
+
+Keep in mind
+
+- Articulate, concise.
+- Provide simple API.
+- Have precise comment.
+- Readable, top-down code.
+
+* References
+
+- Effective Go: [[https://golang.org/doc/effective_go.html][https://golang.org/doc/effective_go.html]]
+- standard package:  [[https://golang.org/pkg/][https://golang.org/pkg/]]
+- Code Review Comments:  [[https://golang.org/s/comments][https://golang.org/s/comments]]
+
+- Go for gophers: [[http://talks.golang.org/2014/go4gophers.slide][http://talks.golang.org/2014/go4gophers.slide]]
+- What's in a name? [[http://talks.golang.org/2014/names.slide][http://talks.golang.org/2014/names.slide]]
+- Organizing Go code: [[http://talks.golang.org/2014/organizeio.slide][http://talks.golang.org/2014/organizeio.slide]]
+
+.image readability/ref.png
+.caption _Gopher_ by [[http://www.reneefrench.com][Renée French]]
diff --git a/2014/readability/close-cond-bad.go b/2014/readability/close-cond-bad.go
new file mode 100644
index 0000000..f7d6774
--- /dev/null
+++ b/2014/readability/close-cond-bad.go
@@ -0,0 +1,26 @@
+package sample // OMIT
+type Stream struct {
+	// some fields
+	isConnClosed     bool
+	connClosedCond   *sync.Cond
+	connClosedLocker sync.Mutex
+}
+
+func (s *Stream) Wait() error {
+	s.connClosedCond.L.Lock()
+	for !s.isConnClosed {
+		s.connClosedCond.Wait()
+	}
+	s.connClosedCond.L.Unlock()
+	// some code
+}
+func (s *Stream) Close() {
+	// some code
+	s.connClosedCond.L.Lock()
+	s.isConnClosed = true
+	s.connClosedCond.L.Unlock()
+	s.connClosedCond.Broadcast()
+}
+func (s *Stream) IsClosed() bool {
+	return s.isConnClosed
+}
diff --git a/2014/readability/close-cond-good.go b/2014/readability/close-cond-good.go
new file mode 100644
index 0000000..f4f7c6d
--- /dev/null
+++ b/2014/readability/close-cond-good.go
@@ -0,0 +1,22 @@
+package sample // OMIT
+type Stream struct {
+	// some fields
+	cc chan struct{} // HL
+}
+
+func (s *Stream) Wait() error {
+	<-s.cc
+	// some code
+}
+func (s *Stream) Close() {
+	// some code
+	close(s.cc)
+}
+func (s *Stream) IsClosed() bool {
+	select {
+	case <-s.cc:
+		return true
+	default:
+		return false
+	}
+}
diff --git a/2014/readability/err_close_write_bad.go b/2014/readability/err_close_write_bad.go
new file mode 100644
index 0000000..733617a
--- /dev/null
+++ b/2014/readability/err_close_write_bad.go
@@ -0,0 +1,16 @@
+package sample // OMIT
+
+func run() error {
+	in, err := os.Open(*input)
+	if err != nil {
+		return err
+	}
+	defer in.Close()
+
+	out, err := os.Create(*output)
+	if err != nil {
+		return err
+	}
+	defer out.Close() // HL
+	// some code
+}
diff --git a/2014/readability/err_close_write_good.go b/2014/readability/err_close_write_good.go
new file mode 100644
index 0000000..26412dc
--- /dev/null
+++ b/2014/readability/err_close_write_good.go
@@ -0,0 +1,20 @@
+package sample // OMIT
+
+func run() (err error) {
+	in, err := os.Open(*input)
+	if err != nil {
+		return err
+	}
+	defer in.Close()
+
+	out, err := os.Create(*output)
+	if err != nil {
+		return err
+	}
+	defer func() { // HL
+		if cerr := out.Close(); err == nil { // HL
+			err = cerr // HL
+		} // HL
+	}() // HL
+	// some code
+}
diff --git a/2014/readability/err_regexp_bad.go b/2014/readability/err_regexp_bad.go
new file mode 100644
index 0000000..8c0d9f2
--- /dev/null
+++ b/2014/readability/err_regexp_bad.go
@@ -0,0 +1,5 @@
+package sample // OMIT
+
+import "regex" // OMIT
+
+var whitespaceRegex, _ = regexp.Compile("\\s+")
diff --git a/2014/readability/err_regexp_good.go b/2014/readability/err_regexp_good.go
new file mode 100644
index 0000000..de3b609
--- /dev/null
+++ b/2014/readability/err_regexp_good.go
@@ -0,0 +1,5 @@
+package sample // OMIT
+
+import "regex" // OMIT
+
+var whitespaceRegex = regexp.MustCompile(`\s+`)
diff --git a/2014/readability/example_test.go b/2014/readability/example_test.go
new file mode 100644
index 0000000..7c960ca
--- /dev/null
+++ b/2014/readability/example_test.go
@@ -0,0 +1,12 @@
+package binary // OMIT
+
+func ExampleWrite() {
+	var buf bytes.Buffer
+	var pi float64 = math.Pi
+	err := binary.Write(&buf, binary.LittleEndian, pi)
+	if err != nil {
+		fmt.Println("binary.Write failed:", err)
+	}
+	fmt.Printf("% x", buf.Bytes())
+	// Output: 18 2d 44 54 fb 21 09 40
+}
diff --git a/2014/readability/if-else-bad.go b/2014/readability/if-else-bad.go
new file mode 100644
index 0000000..f3a3f3b
--- /dev/null
+++ b/2014/readability/if-else-bad.go
@@ -0,0 +1,11 @@
+package sample // OMIT
+
+func sample() { // OMIT
+	if _, ok := f.dirs[dir]; !ok { // HL
+		f.dirs[dir] = new(feedDir) // HL
+	} else {
+		f.addErr(fmt.Errorf("..."))
+		return
+	}
+	// some code
+} // OMIT
diff --git a/2014/readability/if-else-good.go b/2014/readability/if-else-good.go
new file mode 100644
index 0000000..e01b56e
--- /dev/null
+++ b/2014/readability/if-else-good.go
@@ -0,0 +1,11 @@
+package sample // OMIT
+
+func sample() { // OMIT
+
+	if _, found := f.dirs[dir]; found { // HL
+		f.addErr(fmt.Errorf("..."))
+		return
+	}
+	f.dirs[dir] = new(feedDir) // HL
+	// some code
+} // OMIT
diff --git a/2014/readability/if-switch-bad.go b/2014/readability/if-switch-bad.go
new file mode 100644
index 0000000..01235d8
--- /dev/null
+++ b/2014/readability/if-switch-bad.go
@@ -0,0 +1,17 @@
+package sample // OMIT
+
+func BrowserHeightBucket(s *session.Event) string {
+	browserSize := sizeFromSession(s)
+	if h := browserSize.GetHeight(); h > 0 { // HL
+		browserHeight := int(h)
+		if browserHeight <= 480 { // HL
+			return "small"
+		} else if browserHeight <= 640 { // HL
+			return "medium"
+		} else {
+			return "large"
+		}
+	} else {
+		return "null"
+	}
+}
diff --git a/2014/readability/if-switch-good.go b/2014/readability/if-switch-good.go
new file mode 100644
index 0000000..1a8f682
--- /dev/null
+++ b/2014/readability/if-switch-good.go
@@ -0,0 +1,16 @@
+package sample // OMIT
+
+func BrowserHeightBucket(s *session.Event) string {
+	size := sizeFromSession(s)
+	h := size.GetHeight()
+	switch {
+	case h <= 0: // HL
+		return "null"
+	case h <= 480: // HL
+		return "small"
+	case h <= 640: // HL
+		return "medium"
+	default: // HL
+		return "large"
+	}
+}
diff --git a/2014/readability/implement-interface-bad.go b/2014/readability/implement-interface-bad.go
new file mode 100644
index 0000000..00d5b8d
--- /dev/null
+++ b/2014/readability/implement-interface-bad.go
@@ -0,0 +1,10 @@
+package sample // OMIT
+
+import "scan" // OMIT
+
+// Column writer implements the scan.Writer interface.
+type ColumnWriter struct {
+	scan.Writer // HL
+	tmpDir      string
+	// some other fields
+}
diff --git a/2014/readability/implement-interface-good.go b/2014/readability/implement-interface-good.go
new file mode 100644
index 0000000..0feee09
--- /dev/null
+++ b/2014/readability/implement-interface-good.go
@@ -0,0 +1,11 @@
+package sample // OMIT
+
+import "scan" // OMIT
+
+// ColumnWriter is a writer to write ...
+type ColumnWriter struct {
+	tmpDir string
+	// some other fields
+}
+
+var _ scan.Writer = (*ColumnWriter)(nil) // HL
diff --git a/2014/readability/in-band-error-client.go b/2014/readability/in-band-error-client.go
new file mode 100644
index 0000000..2c12028
--- /dev/null
+++ b/2014/readability/in-band-error-client.go
@@ -0,0 +1,11 @@
+package client // OMIT
+
+func proc(it Iterator) (ret time.Duration) {
+	d := it.DurationAt()
+	if d == duration.Unterminated { // HL
+		ret = -1
+	} else {
+		ret = d
+	}
+	// some code
+}
diff --git a/2014/readability/in-band-error.go b/2014/readability/in-band-error.go
new file mode 100644
index 0000000..21d52af
--- /dev/null
+++ b/2014/readability/in-band-error.go
@@ -0,0 +1,21 @@
+package sample // OMIT
+
+import ( // OMIT
+	"duration" // OMIT
+	"time"     // OMIT
+) // OMIT
+
+// duration.Unterminated = -1 * time.Second
+
+func (it Iterator) DurationAt() time.Duration {
+	// some code
+	switch durationUsec := m.GetDurationUsec(); durationUsec {
+	case -1:
+		return duration.Unterminated // HL
+	case -2:
+		return -2 // HL
+	default:
+		return time.Duation(durationUsec) * time.Microsecond // HL
+	}
+	return -3 // HL
+}
diff --git a/2014/readability/long-line-fold.go b/2014/readability/long-line-fold.go
new file mode 100644
index 0000000..2ea15e0
--- /dev/null
+++ b/2014/readability/long-line-fold.go
@@ -0,0 +1,15 @@
+package sampling
+
+import (
+	servicepb "foo/bar/service_proto"
+)
+
+type SamplingServer struct {
+	// some fields
+}
+
+func (server *SamplingServer) SampleMetrics( // HL
+	sampleRequest *servicepb.Request, sampleResponse *servicepb.Response, // HL
+	latency time.Duration) { // HL
+	// some code
+}
diff --git a/2014/readability/long-line-nofold.go b/2014/readability/long-line-nofold.go
new file mode 100644
index 0000000..e0bfd0a
--- /dev/null
+++ b/2014/readability/long-line-nofold.go
@@ -0,0 +1,13 @@
+package sampling
+
+import (
+	servicepb "foo/bar/service_proto"
+)
+
+type SamplingServer struct {
+	// some fields
+}
+
+func (server *SamplingServer) SampleMetrics(sampleRequest *servicepb.Request, sampleResponse *servicepb.Response, latency time.Duration) { // HL
+	// some code
+}
diff --git a/2014/readability/long-line-short.go b/2014/readability/long-line-short.go
new file mode 100644
index 0000000..4440369
--- /dev/null
+++ b/2014/readability/long-line-short.go
@@ -0,0 +1,13 @@
+package sampling
+
+import (
+	spb "foo/bar/service_proto"
+)
+
+type Server struct {
+	// some fields
+}
+
+func (s *Server) SampleMetrics(req *spb.Request, resp *spb.Response, latency time.Duration) { // HL
+	// some code
+}
diff --git a/2014/readability/nil_error.go b/2014/readability/nil_error.go
new file mode 100644
index 0000000..71f64ed
--- /dev/null
+++ b/2014/readability/nil_error.go
@@ -0,0 +1,18 @@
+package main // OMIT
+
+import "log"
+
+type FooError struct{}
+
+func (e *FooError) Error() string { return "foo error" }
+
+func foo() error {
+	var ferr *FooError // ferr == nil // HL
+	return ferr
+}
+func main() {
+	err := foo()
+	if err != nil { // HL
+		log.Fatal(err)
+	}
+}
diff --git a/2014/readability/nil_interface_en.go b/2014/readability/nil_interface_en.go
new file mode 100644
index 0000000..245515e
--- /dev/null
+++ b/2014/readability/nil_interface_en.go
@@ -0,0 +1,16 @@
+package main // OMIT
+
+import "fmt"
+
+type I interface {
+	Key() string
+	Value() string
+}
+type S struct{ I }      // S has method sets of I.
+func (s S) Key() string { return "type S" }
+
+func main() {
+	var s S
+	fmt.Println("key", s.Key())
+	fmt.Println(s.Value()) // runtime error: invalid memory address or nil pointer deference  // HL
+}
diff --git a/2014/readability/reflect-bad.go b/2014/readability/reflect-bad.go
new file mode 100644
index 0000000..6513e84
--- /dev/null
+++ b/2014/readability/reflect-bad.go
@@ -0,0 +1,20 @@
+package sample // OMIT
+
+type Layers struct {
+	UI, Launch /* more fields */ string
+}
+
+func sample() { // OMIT
+	layers := NewLayers(s.Entries)
+	v := reflect.ValueOf(*layers) // HL
+	r := v.Type()                 // type Layers  // HL
+	for i := 0; i < r.NumField(); i++ {
+		if e := v.Field(i).String(); e != "-" {
+			eid := &pb.ExperimentId{
+				Layer:        proto.String(r.Field(i).Name()),
+				ExperimentId: &e,
+			}
+			experimentIDs = append(experimentIDs, eid)
+		}
+	}
+} // OMIT
diff --git a/2014/readability/reflect-good.go b/2014/readability/reflect-good.go
new file mode 100644
index 0000000..2010a2c
--- /dev/null
+++ b/2014/readability/reflect-good.go
@@ -0,0 +1,24 @@
+package sample // OMIT
+
+type LayerExperiment struct{ Layer, Experiment string } // HL
+
+func (t *Layers) Slice() []LayerExperiment { // HL
+	return []LayerExperiment{
+		{"UI", t.UI},
+		{"Launch", t.Launch},
+		/* more fields */
+	}
+}
+
+func sample() { // OMIT
+	layers := NewLayers(s.Entries).Slice() // HL
+	for _, l := range layers {
+		if l.Experiment != "-" {
+			eid := &pb.ExperimentId{
+				Layer:        proto.String(l.Layer),
+				ExperimentId: proto.String(l.Experiment),
+			}
+			experimentIDs = append(experimentIDs, eid)
+		}
+	}
+} // OMIT
diff --git a/2014/readability/resthandler-fix2.go b/2014/readability/resthandler-fix2.go
new file mode 100644
index 0000000..29177b7
--- /dev/null
+++ b/2014/readability/resthandler-fix2.go
@@ -0,0 +1,20 @@
+package resthandler // OMIT
+
+func finishStatus(r Result, complete bool) int {
+	if !complete {
+		return http.StatusAccepted // HL
+	}
+	if stat, ok := r.Object.(*api.Status); ok && stat.Code != 0 {
+		return stat.Code // HL
+	}
+	if r.Created {
+		return http.StatusCreated // HL
+	}
+	return http.StatusOK // HL
+}
+
+func (h *RESTHandler) finishReq(op *Operation, w http.ResponseWriter, req *http.Request) {
+	result, complete := op.StatusOrResult()
+	status := finishStatus(result, complete)     // HL
+	writeJSON(status, h.codec, result.Object, w) // HL
+}
diff --git a/2014/readability/resthandler.go b/2014/readability/resthandler.go
new file mode 100644
index 0000000..e4b97dd
--- /dev/null
+++ b/2014/readability/resthandler.go
@@ -0,0 +1,21 @@
+package resthandler // OMIT
+
+func (h *RESTHandler) finishReq(op *Operation, req *http.Request, w http.ResponseWriter) {
+	result, complete := op.StatusOrResult()
+	obj := result.Object
+	if complete {
+		status := http.StatusOK // HL
+		if result.Created {
+			status = http.StatusCreated // HL
+		}
+		switch stat := obj.(type) {
+		case *api.Status:
+			if stat.Code != 0 {
+				status = stat.Code // HL
+			}
+		}
+		writeJSON(status, h.codec, obj, w) // HL
+	} else {
+		writeJSON(http.StatusAccepted, h.codec, obj, w) // HL
+	}
+}
diff --git a/2014/readability/struct-field-bad.go b/2014/readability/struct-field-bad.go
new file mode 100644
index 0000000..fd3306e
--- /dev/null
+++ b/2014/readability/struct-field-bad.go
@@ -0,0 +1,8 @@
+package sample // OMIT
+
+type Modifier struct {
+	pmod   *profile.Modifier
+	cache  map[string]time.Time
+	client *client.Client
+	mu     sync.RWMutex // HL
+}
diff --git a/2014/readability/struct-field-good.go b/2014/readability/struct-field-good.go
new file mode 100644
index 0000000..7cab133
--- /dev/null
+++ b/2014/readability/struct-field-good.go
@@ -0,0 +1,9 @@
+package sample // OMIT
+
+type Modifier struct {
+	client *client.Client
+
+	mu    sync.RWMutex // HL
+	pmod  *profile.Modifier
+	cache map[string]time.Time
+}
diff --git a/2014/readability/test-pattern_en.go b/2014/readability/test-pattern_en.go
new file mode 100644
index 0000000..5915d9e
--- /dev/null
+++ b/2014/readability/test-pattern_en.go
@@ -0,0 +1,8 @@
+package sample // OMIT
+
+func TestSample(t *testing.T) { // OMIT
+	// Typical test code
+	if got, want := testTargetFunc(input), expectedValue; !checkTestResult(got, want) {
+		t.Errorf("testTargetFunc(%v) = %v; want %v", input, got, want)
+	}
+} // OMIT
diff --git a/2014/readability/time_duration_bad.go b/2014/readability/time_duration_bad.go
new file mode 100644
index 0000000..4db3c24
--- /dev/null
+++ b/2014/readability/time_duration_bad.go
@@ -0,0 +1,3 @@
+package sample // OMIT
+
+var rpcTimeoutSecs = 30 // Thirty seconds  // HL
diff --git a/2014/readability/time_duration_bad1.go b/2014/readability/time_duration_bad1.go
new file mode 100644
index 0000000..d003ca4
--- /dev/null
+++ b/2014/readability/time_duration_bad1.go
@@ -0,0 +1,5 @@
+package sample // OMIT
+
+import "time" // OMIT
+
+var rpcTimeout = time.Duration(30 * time.Second) // Thirty seconds  // HL
diff --git a/2014/readability/time_duration_bad2.go b/2014/readability/time_duration_bad2.go
new file mode 100644
index 0000000..620edb2
--- /dev/null
+++ b/2014/readability/time_duration_bad2.go
@@ -0,0 +1,5 @@
+package sample // OMIT
+
+import "time" // OMIT
+
+var rpcTimeout = time.Duration(30) * time.Second // Thirty seconds  // HL
diff --git a/2014/readability/time_duration_good.go b/2014/readability/time_duration_good.go
new file mode 100644
index 0000000..b99cd1d
--- /dev/null
+++ b/2014/readability/time_duration_good.go
@@ -0,0 +1,5 @@
+package sample // OMIT
+
+import "time" // OMIT
+
+var rpcTimeout = 30 * time.Second // HL
diff --git a/2014/readability/val-and-error.go b/2014/readability/val-and-error.go
new file mode 100644
index 0000000..f6ddc01
--- /dev/null
+++ b/2014/readability/val-and-error.go
@@ -0,0 +1,25 @@
+package sample // OMIT
+
+import ( // OMIT
+	"errors" // OMIT
+	"time"   // OMIT
+) // OMIT
+
+var (
+	ErrDurationUnterminated = errors.new("duration: unterminated")
+	ErrNoDuration           = errors.New("duration: not found")
+	ErrNoIteration          = errors.New("duration: not interation")
+)
+
+func (it Iterator) DurationAt() (time.Duration, error) { // HL
+	// some code
+	switch durationUsec := m.GetDurationUsec(); durationUsec {
+	case -1:
+		return 0, ErrDurationUnterminated // HL
+	case -2:
+		return 0, ErrNoDuration // HL
+	default:
+		return time.Duation(durationUsec) * time.Microsecond, nil // HL
+	}
+	return 0, ErrNoIteration // HL
+}