Skip to main content

Go Best Practices & Code Review Checklist

สรุปจาก Effective Go, Go Code Review Comments, และ pattern ที่เจอบ่อยในโค้ด production

Error Handling​

  • ห้าม swallow error — ไม่ _ = doSomething() ทิ้งเฉยๆ ยกเว้นมีคอมเมนต์อธิบายว่าทำไมปลอดภัย
  • Wrap error ด้วย context ที่มีประโยชน์: fmt.Errorf("fetch user %s: %w", id, err) — ไม่ใช่แค่ fmt.Errorf("error: %w", err) ที่ไม่บอกอะไรเพิ่ม
  • Sentinel error (var ErrNotFound = errors.New(...)) ใช้เมื่อ caller ต้องเช็ค "ชนิด" ของ error ส่วน custom error type (struct ที่ implement Error()) ใช้เมื่อต้องพก field เพิ่ม (เช่น field ไหนที่ validate ไม่ผ่าน)
  • อย่า panic ใน library code — panic เหมาะกับ "โปรแกรมเมอร์เขียนผิด" (เช่น index out of range) ไม่ใช่ "runtime error ที่คาดเดาได้" (เช่น network timeout) ซึ่งควร return error
🧠 จำง่ายๆ

"Sentinel = ป้ายชื่อ, Custom type = ซองจดหมายมีของแถม" — sentinel error เหมือนป้ายชื่อที่แค่ บอกว่า "คืออันนี้" (errors.Is) ส่วน custom error type เหมือนซองจดหมายที่มีของแถมข้างใน (field พิเศษให้ดึงออกมาใช้ด้วย errors.As) ถ้าแค่ต้องรู้ "ชนิด" ใช้ป้ายชื่อพอ ถ้าต้องรู้ "รายละเอียดเพิ่ม" ต้องแกะซอง

🎯 Beyond: จุดที่ชาวบ้านไม่ค่อยรู้

Sentinel error ที่ประกาศแบบ var ErrNotFound = errors.New("not found") มีข้อเสียที่คนมักมองข้าม: ทุก package ที่ต้องการ sentinel ตัวนี้ต้อง import package ที่ประกาศมันไว้ ทำให้เกิด coupling ที่ไม่จำเป็นในระบบใหญ่ แนวทางที่ mature กว่าคือใช้ custom error type ที่ implement Is(target error) bool เอง เพื่อให้ errors.Is เทียบแบบ custom logic ได้โดยไม่ต้อง share ตัวแปร sentinel ข้าม package เลย:

type NotFoundError struct{ Resource string }
func (e *NotFoundError) Error() string { return e.Resource + " not found" }
func (e *NotFoundError) Is(target error) bool {
    _, ok := target.(*NotFoundError)
    return ok
}

เป็นเทคนิคที่ library ระดับ production จำนวนมากใช้ แต่น้อยคนจะรู้ว่า error interface รองรับ Is() แบบ custom ได้ด้วย

Naming Conventions​

  • Exported identifier ขึ้นต้นตัวใหญ่ (UserService), unexported ตัวเล็ก (userCache)
  • ชื่อสั้นในสโคปแคบ (i ใน loop, err เสมอสำหรับ error) ชื่อยาวขึ้นตามสโคปที่กว้างขึ้น
  • Getter ไม่ใส่ Get prefix: user.Name() ไม่ใช่ user.GetName()
  • Interface ที่มี method เดียวมักลงท้ายด้วย -er: Reader, Writer, Closer, Stringer
🧠 จำง่ายๆ

"สโคปแคบ ชื่อสั้น, สโคปกว้าง ชื่อยาว" — คิดเป็นกราฟเส้นตรง: ยิ่งตัวแปรมีชีวิตอยู่สั้น (เห็นแค่ 2-3 บรรทัดใน loop) ยิ่งตั้งชื่อสั้นได้ (i, v) ยิ่งตัวแปร/ฟังก์ชันมีคนเห็น/ใช้เยอะ (exported ทั้ง package) ยิ่งต้องตั้งชื่อยาวให้สื่อความหมายชัดเจน เพราะคนอ่านไม่มี context รอบข้างช่วย

🎯 Beyond: จุดที่ชาวบ้านไม่ค่อยรู้

กฎ "ไม่ใส่ Get prefix" มีข้อยกเว้นที่คนมักไม่รู้: ถ้า method ชื่อเดียวกับ field ไม่ได้ (ชนกัน ไม่ได้เพราะ field เป็น unexported อยู่แล้วและ method ต้อง export) แต่ compile ไม่ผ่านจริงๆ เมื่อ field กับ method ชื่อเดียวกันในสโคปเดียวกัน ถึงจะคนละ case (ตัวใหญ่/เล็ก) ก็ตาม เพราะ Go ไม่สน case ในการตรวจชนกันของชื่อใน struct เดียวกัน ต่างจากภาษาอื่นที่ getName() กับ name อยู่ด้วยกันได้สบายๆ — เป็นเหตุผลจริงที่ Go ไม่มี Get prefix แพร่หลาย ไม่ใช่แค่ สไตล์ส่วนตัว

Concurrency Safety​

  • ทุก goroutine ที่สร้างต้องมีคนรอ (WaitGroup) หรือมี context ที่ cancel ได้ — ห้ามปล่อยลอย ("fire and forget" คือสาเหตุ goroutine leak อันดับ 1)
  • ห้าม mutate shared state โดยไม่ล็อก — รัน go test -race ใน CI เสมอ
  • ปิด channel โดยฝั่ง sender เท่านั้น (ปิดโดย receiver แล้ว sender ส่งต่อจะ panic)
🧠 จำง่ายๆ

"ใครสร้าง goroutine คนนั้นต้องรับผิดชอบจนจบ" — เหมือนจุดไฟแล้วต้องดับเอง ห้ามจุดแล้วเดินหนี (fire-and-forget) ทุก go func() ต้องมีคำตอบชัดเจนว่า "ใครรอมันจบ" หรือ "อะไรทำให้มันหยุดได้"

🎯 Beyond: จุดที่ชาวบ้านไม่ค่อยรู้

"ปิด channel โดย sender เท่านั้น" เป็นกฎที่ถูกแค่ครึ่งเดียวในระบบจริง — กฎที่แม่นยำกว่าคือ "ให้ owner ของ channel เป็นคนปิด และ owner ต้องเป็นฝ่ายเดียวที่รู้ว่า sender ทุกตัวส่งจบแล้ว" ถ้ามี sender หลายตัว ไม่มี sender ตัวไหนควรปิด channel เองโดยลำพัง (เพราะไม่รู้ว่า sender อื่น ส่งจบหรือยัง) ต้องมี goroutine กลาง (มักเป็นตัวที่สร้าง channel เอง) คอยนับด้วย WaitGroup แล้วเป็นคนเดียวที่เรียก close() หลังจาก sender ทุกตัว Done() ครบ — "sender ปิด" ใช้ได้จริง เฉพาะกรณี sender ตัวเดียว เท่านั้น

Performance​

  • หลีกเลี่ยง unnecessary allocation: ใช้ strings.Builder แทนการต่อ string ด้วย + ใน loop
  • Preallocate slice/map เมื่อรู้ขนาดล่วงหน้า: make([]T, 0, n) ลด reallocation
  • Pass struct ใหญ่เป็น pointer เพื่อลดการ copy แต่ struct เล็ก (ไม่กี่ field) ส่งเป็น value ได้เร็วกว่า (หลีกเลี่ยง heap allocation + indirection)
  • Profile ก่อน optimize เสมอ: pprof (go tool pprof) — "premature optimization is the root of all evil" ยังใช้ได้กับ Go
🧠 จำง่ายๆ

"string + string ใน loop = ก่อกำแพงอิฐใหม่ทุกก้อน" — Go string เป็น immutable ทุกครั้งที่ + จะสร้าง string ใหม่ทั้งก้อน (ไม่ได้ต่อท้ายของเดิม) ยิ่งวนเยอะยิ่งเปลืองเท่าทวีคูณ strings.Builder เหมือนมีถังปูนไว้ค่อยๆ เติมอิฐ แล้วเทพิมพ์เป็นกำแพงจริงครั้งเดียวตอนจบ

🎯 Beyond: จุดที่ชาวบ้านไม่ค่อยรู้

make([]T, 0, n) (length 0, capacity n) ต่างจาก make([]T, n) (length n) อย่างสำคัญที่คน มักสับสน: แบบแรกต้อง append เพื่อเติมของเข้าไป (ปลอดภัย ไม่มี zero value ปน) แบบหลังสร้าง element ว่าง n ตัวทันที (เป็น zero value ทั้งหมด) ถ้าใช้ append กับแบบหลังจะเติมต่อท้าย element ว่างที่มีอยู่แล้ว ไม่ใช่เขียนทับ เป็นบั๊กที่พบบ่อยมากเวลา preallocate แล้วลืมว่าตัวเอง สร้าง length เท่าไหร่ไว้ — กฎจำง่ายๆ: ถ้าจะใช้ append ต่อ ให้ preallocate ด้วย capacity (make([]T, 0, n)) เสมอ ไม่ใช่ length

Code Review Checklist​

  • ทุก error ถูกเช็คหรือ wrap อย่างมีเหตุผล (ไม่มี _ = err)
  • ไม่มี goroutine ที่ leak (มี cancellation path ชัดเจน)
  • context.Context เป็น parameter แรก และไม่ถูกเก็บใน struct
  • Exported function/type มี doc comment ขึ้นต้นด้วยชื่อตัวเอง (// UserService handles...)
  • ไม่มี magic number — ใช้ named constant
  • Table-driven test ครอบคลุม edge case (empty input, nil, boundary value)
  • ไม่มี interface{}/any ที่ใช้พร่ำเพรื่อโดยไม่จำเป็น (เสีย type safety)
  • go vet และ golangci-lint ผ่านแบบไม่มี warning ที่ไม่ได้ตั้งใจ suppress

เครื่องมือที่ควรมีใน CI​

go vet ./...
staticcheck ./...
golangci-lint run
go test -race -cover ./...
🧠 จำง่ายๆ

"vet จับโค้ดที่ compile ผ่านแต่ผิด, staticcheck จับโค้ดที่ compile ผ่านแต่ไม่ฉลาด" — go vet เน้นจับ bug ที่ compiler ธรรมดาไม่จับ (เช่น Printf ที่ format string ไม่ตรงกับ argument) staticcheck เน้นจับ code smell/anti-pattern ที่ compile ผ่านสบายๆ แต่ไม่ใช่ วิธีที่ควรเขียน (เช่น error ที่ไม่เคย check เงื่อนไข, unused code ที่ชับซ้อน)

🎯 Beyond: จุดที่ชาวบ้านไม่ค่อยรู้

go vet ถูกรันเป็นส่วนหนึ่งของ go test โดยอัตโนมัติอยู่แล้ว (ตั้งแต่ Go 1.10) โดยไม่ต้อง สั่งแยก — หลายทีมใส่ go vet ./... เป็น step แยกใน CI ทั้งที่จริงๆ ซ้ำซ้อนกับที่ go test รันให้อยู่แล้ว (ยกเว้นต้องการรัน vet แยกโดยไม่รัน test เช่นตรวจเร็วๆ ก่อน) ส่วน go vet ชุด check ที่รันตอน go test เป็นแค่ subset ของ check ทั้งหมดที่ go vet มี (เลือกเฉพาะตัวที่ "แม่นยำสูง false positive ต่ำ") ถ้าต้องการ check ครบทุกตัวต้องรัน go vet ./... แยกต่างหาก จริง

English Summary

Never silently discard an error; wrap it with actionable context using %w. Use sentinel errors for type checks, custom error structs when you need extra fields — and for decoupled sentinel- style checks across packages, implement a custom Is(target error) bool method instead of sharing a package-level variable. Reserve panic for programmer bugs, not expected runtime failures. Every goroutine needs an owner that waits for it or a cancellable context — unbounded "fire and forget" goroutines are the most common leak, and only a single sender should ever close a channel (multi-sender cases need a dedicated coordinator). Prefer strings.Builder and preallocated slices/maps over naive concatenation/growth — remember make([]T, 0, n) (ready for append) differs from make([]T, n) (already has n zero-valued elements). Always profile with pprof before optimizing. go vet already runs as part of go test automatically since Go 1.10 — a separate CI step only matters if you want its full check set, not the safe subset used during testing.