Go Code Review Comments 정리

laeshiny
13 min readFeb 18, 2018

--

Gofmt

Comment Sentences

참고 : https://golang.org/doc/effective_go.html#commentary

  • comment는 완전한 문장으로 작성하세요. 나중에 godoc으로 문서화할 때 좋습니다.
  • comment는 설명할 대상의 이름으로 시작하세요.
// 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) { ...

Contexts

  • context.Context 타입은 보안 정보, 분석 정보, 데드라인, 취소 신호 그리고 프로세스 경계 정보를 가집니다.
  • 대부분 함수 첫 번째 인자로 전달합니다.
func F(ctx context.Context, /* other arguments */) {}
  • Context를 구조체에 넣지 말고 함수 인자로 전달해서 사용하세요.
  • Custom Context type 만들어서 사용하지 마세요.
  • Context는 immutable 하니 같은 데드라인, 취소 신호, 보안 정보, parent trace 등 다양한 call에 사용해도 좋아요.

Copying

  • 예기치 않은 재명명을 피하려면 다른 패키지에 있는 구조체를 복사할 때 주의해야 합니다.
  • byte.Buffer를 복사하면 복사본에 있는 슬라이스는 원본 배열에 별칭이 될 수 있기 때문에 그 뒤에 따라오는 메소드 호출에 영향을 줄 수 있습니다.
  • 일반적으로 메소드가 포인터 타입 *T와 연관되어 있다면 타입 T에 값을 복사하지 마십시오.

Declaring Empty Slices

  • empty slice를 선언할 때 var t []string 를 t := []string{} 대신 사용하세요.
  • 전자는 nil slice 값을, 후자는 non-nil에 len과 cap이 0인 값을 가집니다.
  • JSON 오브젝트를 대룰 때는 전자는 null, 후자는 [] array를 가집니다. JSON 한정에서는 후자가 더 낫습니다.

Crypto Rand

  • key를 만들기 위해 math/rand 를 사용하지 말고crypto/rand 를 사용하세요.
  • time.Nanoseconds()에 seed가 있더라도 단지 몇 비트의 엔트로피가 있습니다.
import (
"crypto/rand"
// "encoding/base64"
// "encoding/hex"
"fmt"
)

func Key() string {
buf := make([]byte, 16)
_, err := rand.Read(buf)
if err != nil {
panic(err) // out of randomness, should never happen
}
return fmt.Sprintf("%x", buf)
// or hex.EncodeToString(buf)
// or base64.StdEncoding.EncodeToString(buf)
}

Doc Comments

Don’t Panic

Error Strings

  • error string은 소문자로 시작하세요.
  • fmt.Errorf(“Something bad”) 말고 fmt.Errorf(“something bad”) 를 사용하세요.

Examples

Goroutine Lifetimes

  • 고루틴을 생성한 뒤 혹은 종료할 때 정리 작업을 해주세요.
  • 고루틴은 채널에 데이터를 보내거나 받을 때 블럭킹이 되어 누수를 발생시킬 수 있습니다.
  • 고루틴을 사용하지 않고, 문제 없어 보여도 나중에 문제를 일으킬 수 있습니다.
  • 동시성 코드는 고루틴 수명을 명확히 해서 단순화게 짜도록 합니다. 그렇게 하기 힘들면 언제 그리고 왜 고루틴이 종료하는지 문서화를 합니다.

Handle Errors

Imports

  • import package 이름이 겹치는 경우를 제외하고는 이름을 바꾸지 마세요.
  • import는 아래와 같은 방식으로 그룹화를 하세요.
package main

import (
"fmt"
"hash/adler32"
"os"

"appengine/foo"
"appengine/user"

"github.com/foo/bar"
"rsc.io/goversion/version"
)

Import Dot

  • import . 은 해당 패키지로 인식시키기 때문에 테스트할 때 유용한데, 그 외에는 혼란을 줄 수 있으니 테스트할 때만 사용하세요.
package foo_test

import (
"bar/testutil" // also imports "foo"
. "foo"
)

In-Band Errors

  • Go는 여러개의 반환 값을 지원하기 때문에 client가 반환 값을 검사해서 오류 검사를 하도록 추가적인 값을 반환하도록 하는게 좋습니다.
// Lookup returns the value for key or ok=false if there is no mapping for key.
func Lookup(key string) (value string, ok bool)
value, ok := Lookup(key)
if !ok {
return fmt.Errorf("no value for %q", key)
}
return Parse(value)

Indent Error Flow

  • indentation을 최소화하면 가독성이 좋기 때문에 아래 보다는
if err != nil {
// error handling
} else {
// normal code
}
  • 아래와 같은 식으로 짜는게 좋습니다.
if err != nil {
// error handling
return // or continue, etc.
}
// normal code
  • 초기화 코드에서도 아래 보다는
if x, err := f(); err != nil {
// error handling
return
} else {
// use x
}
  • 아래처럼 짜는게 좋습니다.
x, err := f()
if err != nil {
// error handling
return
}
// use x

Initialisms

  • 두문자어는 대문자로 합니다. 예를 들어 Url 보다는 URL, ServeHttp 보다는 ServeHTTP 를 사용하도록 합니다.

Interfaces

  • interface는 구현하는 패키지가 아닌 interface type 값을 사용하는 패키지에 속에 속합니다.
  • interface를 “mocking” 위해 api 구현체 쪽에 정의를 하지 말고 api가 실제 구현한 공개 api를 사용해서 테스트를 할 수 있도록 설계해야 합니다.
  • interface를 사용되기 전에 정의하지 마세요. 실제 사용 예가 없이 인터페이스가 필요한지 여부를 알기 힘듭니다.
package consumer  // consumer.go

type Thinger interface { Thing() bool }

func Foo(t Thinger) string { … }
====================================================
package consumer // consumer_test.go

type fakeThinger struct{ … }
func (t fakeThinger) Thing() bool { … }

if Foo(fakeThinger{…}) == "x" { … }
====================================================
// DO NOT DO IT!!!
package producer

type Thinger interface { Thing() bool }

type defaultThinger struct{ … }
func (t defaultThinger) Thing() bool { … }

func NewThinger() Thinger { return defaultThinger{ … } }
====================================================
//대신에 생성자 타입을 반환하고 컨슈머가 프로듀서 구현자를 목킹할 수 있도록 하는게 좋습니다. package producer

type Thinger struct{ … }
func (t Thinger) Thing() bool { … }

func NewThinger() Thinger { return Thinger{ … } }

Line Length

  • Go는 엄격한 줄 길이 제한이 없지만 불필요한 긴 코드는 피해야 합니다.
  • 예를 들어 긴 코드가 더 가독성이 좋다면 줄 길이를 짧게 하기 위해 줄을 끊지 마세요.
  • “N 줄보다 긴 함수를 절대 사용하지 마십시오” 같은 규칙은 없습니다. 줄 길이 보다는 의미에 집중하세요.

Mixed Caps

Named Result Parameters

  • named result parameter를 사용하면 godoc 에서 보기 안 좋을 수 있습니다.
  • 하지만 비슷한 type이 연달아 있거나 의미상 변수 이름을 넣는게 의도 파악을 더 하기 좋다면 사용하는게 좋습니다.

Naked Returns

Package Comments

  • 패키지 주석은 패키지 바로 위에 적어주세요.
// Package math provides basic constants and mathematical functions.
package math
  • “pakcage main”에 대한 주석을 위해 Binary … Command … 같은 방식도 괜찮습니다.
// Binary seedgen ...
package main

or

// Command seedgen ...
package main

Package Names

Pass Values

  • 단순히 몇 바이트를 아끼기 위해서 포인터를 함수 인자로 넘기지 마세요.
  • 필요도 없이 함수 전반적으로 *x 와 같은 방식을 사용할 거면 포인터가 필요 없습니다.
  • 물론 큰 구조체나 앞으로 커질 구초제에는 해당사항이 아닙니다.

Receiver Names

  • 리시버 이름은 하나 혹은 두 문자로 축약해서 이름을 짓습니다.
  • “me”, “this”, “self” 같은 전형적인 객체지향 언어들에서 사용하는 포괄적인 이름을 사용하지 마세요.
  • 리시버는 메소드 인자만큼 서술적일 필요가 없고 문서화를 위한 목적으로도 사용하지 않습니다.
  • 그리고 모든 메소드에 걸쳐서 사용될 수 있으니 짧은 게 좋습니다.

Receiver Type

리시버 타입을 포인터로 할지 값으로 할지 고민이 되면 아래 가이드를 참고하세요.

  • 리시버가 맵, 함수 또는 채널인 경우 포인터를 사용하지 마세요
  • 리시버가 슬라이스고 메소드가 리슬라이스나 재할당을 하지 않는 경우 포인터를 사용하지 마세요
  • 메소드가 리시버를 변경할 필요가 있다면 포인터를 사용하세요
  • 리시버가 구조체고 sync.Mutex 같은 동기화 필드를 가지고 있다면 복사를 피하기 위해 포인터를 사용해야 합니다.
  • 리시버가 사이즈가 큰 구조체나 배열이라면 포인터를 사용하는게 효율적입니다.
  • 함수나 메소드가 동시에 혹은 메소드에서 호출될 때 리시버를 수정할 수 있습니까? 리비서를 값으로 전달하는 방식은 메소드가 호출될 때 리시버가 복사됩니다. 그래서 리시버를 사용하는 다른 곳에도 영향을 주려면 포인터를 사용하십시오.
  • 리시버가 구조체, 배열 혹은 슬라이스이거나 포인터를 요소로 가지고 있다면 포인터를 사용하세요.
  • 리시버가 작은 배열이나 구조체라면 값 리시버가 낫습니다. value 메소드에 value가 전달이 된다면 힙을 새로 할당하는 대신 스택에 복사본을 사용할 수 있어서 가바지를 줄일 수 있습니다.
  • 마지막으로, 무엇을 이용할지 불분명하다면 포인터를 사용하세요.

Synchronous Functions

  • 함수가 끝나자마자 바로 반환 값을 돌려주거나, 반환 값을 돌려주기 전에 콜백이나 채널로 데이터를 전달하는 함수들은 비동기보다 동기 함수가 더 좋습니다.
  • 동기 함수는 생존기간을 분석하거나 누수나 데이터 레이스를 막기가 더 쉽습니다. 호출자가 더 많은 동시성을 필요하면 별도의 고루틴에서 함수를 호출하여 쉽게 추가할 수 있습니다.
  • 하지만 호출자 쪽에서 불필요한 동시성을 제거하는 것은 불가능할 때가 있습니다.

Useful Test Failures

  • 테스트가 실패한다면 그 이유를 알 수 있도록 충분한 정보가 있는 에러 메세지를 출력해야 합니다.
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
}

Variable Names

  • Go 에서 변수 이름은 긴 것보다는 짧은 것을 선호합니다. lineCount 보다는 c, sliceIndex 보다는 i 를 더 좋아합니다.
  • 리시버는 한 두 문자면 충분합니다. 필요한 경우에만 좀 더 설명을 붙인 이름을 사용합니다.

--

--

No responses yet