Goの埋め込みstructで責務境界が曖昧な実装をどうレビューするか
Goの埋め込みstructで責務境界が曖昧な実装をどうレビューするか
Goのstruct embeddingは、フィールド名を書かずに型を埋め込める便利な機能だ。
メソッド昇格により、委譲コードを減らせる場面もある。
ただしレビューでは、依存注入やコード共有のために安易に埋め込みを使っていないかを慎重に見る必要がある。
埋め込みは、内部の依存やメソッドを外側の型の能力として見せてしまうからだ。
この記事では、埋め込みstructの使い方ではなく、
責務境界と公開APIが曖昧になっていないかをレビューする観点を整理する。
まず止めたい実装
type UserService struct {
*sql.DB
Logger
}
func (s *UserService) CreateUser(ctx context.Context, name string) error {
_, err := s.ExecContext(ctx, "INSERT INTO users(name) VALUES(?)", name)
if err != nil {
s.Error("create user failed", err)
return err
}
return nil
}
@Reviewer`*sql.DB` と `Logger` が埋め込まれており、UserServiceの公開能力としてDB操作やログ操作が昇格しています。依存は名前付きフィールドにして、この型が提供するAPIを意図的に絞ってください。
このコードでは、UserService が ExecContext や QueryContext を直接持つ型のように見える。
利用者は service.QueryContext(...) を呼べてしまうため、サービスの責務境界が崩れる。
レビューで見るべきなのは、埋め込みで行数が減ったかどうかではない。
外側の型が何を公開するべきかである。
なぜ危ないのか
struct embeddingは、フィールドを持つだけではなく、埋め込んだ型のメソッドを外側の型へ昇格させる。
そのため、意図しない公開APIが増えやすい。
実務では次のような問題につながる。
- サービス層からDBの任意操作が見えてしまう
- ロガーや設定のメソッドが業務型の能力として見える
- テストで差し替えるべき依存範囲が広がる
- 将来埋め込み型にメソッドが増えたとき外側のAPIも増える
- 「継承の代用品」のように使われ、責務が混ざる
Goに継承はないが、埋め込みを雑に使うと、継承に近い曖昧さを持ち込むことがある。
レビューで見たい3つの判断線
1. 依存注入のためだけに埋め込んでいないか
DB、Logger、Config、HTTP Clientなどの依存を埋め込むと、外側の型のAPIとして見えてしまう。
依存を保持したいだけなら、名前付きフィールドで十分なことが多い。
@Reviewer: この埋め込みは依存注入の省略記法として使われています。外側の型に依存先のメソッドを公開する必要がないなら、名前付きフィールドにして責務境界を明確にしてください。2. メソッド昇格が利用者にとって自然な能力か
埋め込みが妥当かは、昇格したメソッドを外側の型のメソッドとして呼べることが自然かで判断する。
@Reviewer: `UserService.QueryContext` と呼べる構造になっていますが、UserServiceはDBそのものではありません。昇格メソッドが利用者に誤った能力を見せていないか確認してください。3. 将来のメソッド追加で外側のAPIが変わらないか
埋め込まれた型に新しいメソッドが追加されると、外側の型にもそのメソッドが見える場合がある。
レビューでは、この影響範囲を見落とさない。
@Reviewer: 埋め込み型のメソッド集合が外側の公開APIに影響します。将来の依存型変更でUserServiceのAPIが増減する構造は避け、明示的なメソッドだけを公開してください。改善例:名前付きフィールドで依存を閉じる
依存を持たせたいだけなら、名前付きフィールドにする。
type UserService struct {
db *sql.DB
logger Logger
}
func NewUserService(db *sql.DB, logger Logger) *UserService {
return &UserService{
db: db,
logger: logger,
}
}
func (s *UserService) CreateUser(ctx context.Context, name string) error {
_, err := s.db.ExecContext(ctx, "INSERT INTO users(name) VALUES(?)", name)
if err != nil {
s.logger.Error("create user failed", err)
return err
}
return nil
}この形なら、UserService の利用者は CreateUser だけを見る。
DB操作やログ操作は内部実装として閉じる。
レビュー観点としても次が明確になる。
- 公開APIが意図したメソッドだけになる
- 依存の所有者が分かる
- テストで差し替える対象を絞りやすい
- DB操作をサービス外から呼ばせない
埋め込みが妥当なケース
埋め込みが有効な場面もある。
例えば、型の振る舞いを意図的に拡張し、外側の型が内側の能力を自然に持つ場合だ。
type AuditBuffer struct {
bytes.Buffer
}
func (b *AuditBuffer) WriteAuditLine(line string) error {
_, err := b.WriteString(time.Now().Format(time.RFC3339) + " " + line + "\n")
return err
}この例では、AuditBuffer がbufferとして振る舞うことが自然であり、Write や String を持つことも説明しやすい。
それでもレビューでは、外側の型が内側の全メソッドを公開してよいかを確認する。
よくある危険パターン
Configの埋め込み
type Server struct {
Config
router *http.ServeMux
}@Reviewer: Configを埋め込むことで、設定値がServerのフィールドとして直接見えます。設定は依存データであり、Serverの公開構造に混ぜる必要があるか再検討してください。共通処理のmixin化
type AdminHandler struct {
BaseHandler
}@Reviewer: BaseHandlerの埋め込みにより、認証、ログ、レスポンス生成など複数責務がAdminHandlerへ昇格しています。共通処理は小さな関数や明示的な依存として分けられないか確認してください。レビュー観点チェックリスト
- 埋め込み型のメソッドを外側の型のAPIとして公開してよいか
- 依存注入の省略として埋め込みを使っていないか
- 外側の型が内側の型として振る舞うことが自然か
- 将来のメソッド追加で公開APIが意図せず増えないか
- テストで差し替える依存範囲が広がっていないか
- 名前付きフィールドでは不十分な理由が説明できるか
まとめ
Goのstruct embeddingは、委譲を簡潔にできる一方で、公開APIと責務境界を曖昧にしやすい。
レビューでは、行数削減よりも 外側の型が何を提供する型なのかを優先して判断したい。
依存を持たせるだけなら、名前付きフィールドが基本である。
埋め込みを使うなら、昇格するメソッドまで含めて外側の型の能力として説明できる必要がある。