GoのLinterさん【calculated cyclomatic complexity】は自分のコード力が低いってこと？ #CI

こんにちは。
株式会社HRBrainでバックエンドエンジニアをしているみつです！

Goで実装をしていてLinterにcalculated cyclomatic complexityと怒られ、しょんぼりしながら記事を書いています

//nolint:したい気持ちを一旦抑えて、「cyclomatic complexity（循環的複雑度）」について調べてCIと戦いたいと思います。

cyclomatic complexity（循環的複雑度）とは？

そもそも循環的複雑度というのは、リファクタリングが必要なコードを特定するためのコード品質指標。

独立な経路の数を数値化するもので、関数のソースコードにおける複雑さを計測する指標です。

サイクロマティック複雑度は、リファクタリングが必要なコードを特定するために使用できるコード品質指標である。関数のソースコードを通る直線的に独立したパスの数を測定する。[DeepL訳]

Cyclomatic complexity is a code quality metric which can be used to identify code that needs refactoring. It measures the number of linearly independent paths through a function's source code.

良い複雑度の指標

10以下だと非常に良い構造とされているようです。

エラーの内容

usecaseが太ってしまいました。

  Running [/home/runner/golangci-lint-1.55.2-linux-amd64/golangci-lint run --out-format=github-actions --path-prefix=xxx/xxx/xxx/ --out-${NO_FUTURE}format colored-line-number] in [/home/runner/work/hrbrain/hrbrain/xxx/xxx/xxx] ...
  
  Error: xxx/xxx/xxx/usecases/xxx.go:56:1: calculated cyclomatic complexity for function Create is 12, max is 10 (cyclop)
  func (u usecase) Create(
  ^

起きたこと

<Create>の関数をあるタスクで実装しました。

その<Create>の関数の中では、特定の条件に一致した時、<Create>するというような記述をしました。

そして、次のタスクで<Create>に加えて、ある条件のときに<特定の処理A>をしたいため、下記のような実装をしました。

しかし、そこでUsecaseが太っていると怒られてしまいました。

ダメな実装.go

func (u Usecase) Create(
	...
) (domain.ID, error) {
	err := u.tx.Transaction(ctx, func(ctx context.Context) error {
		var err error
		if ("条件に一致した時、<Create>と<特定の処理A>をしたい") {
			id, err = "<Create>"
			if err != nil {
				return xerrors.Errorf("failed: %w", err)
			}
   			xxx, err = "<特定の処理A>"
      		if err != nil {
				return xerrors.Errorf("failed: %w", err)
			}
			return nil
		}
  
		isExists, err := "何かしらの処理"
		if err != nil {
			return xerrors.Errorf("failed: %w", err)
		}
		if !isExists {
			return xerrors.Errorf("Not found error")
		}

		xxx, err := "何かしらの処理"
		if err != nil {
			return xerrors.Errorf("failed: %w", err)
		}

		xxx, err = "<Create>"
		if err != nil {
			return xerrors.Errorf("failed: %w", err)
		}
  
  		xxx, err = "<特定の処理A>"
		if err != nil {
			return xerrors.Errorf("failed: %w", err)
		}

		err = "何かしらの処理"
		if err != nil {
			return xerrors.Errorf("failed: %w", err)
		}

		err = "何かしらの処理"
		if err != nil {
			return xerrors.Errorf("failed: %w", err)
		}

		return nil
	})
	if err != nil {
		return domain.ID{}, xerrors.Errorf("failed transaction: %w", err)
	}

	return id, nil
}

やったこと

Lintに怒られて、//nolint:したい気持ちを抑えてゆっくり考えてみます。

ふと、<Create>の処理と<特定の処理A>は共通した処理であることに気が付きました。

そこで考え方を変え、「条件に一致する時、何かをする」のではなく、「条件に一致したらそこまでにする」という書き方に修正を行いました。

こうすることでifの分岐の中でエラーチェックのためのif err != nilがなくなり、階層が浅くなりました

before

if ("条件に一致した時、<Create>と<特定の処理A>をしたい") {
	id, err = "<Create>"
	if err != nil {
		return xerrors.Errorf("failed: %w", err)
	}
	   xxx, err = "<特定の処理A>"
	  if err != nil {
		return xerrors.Errorf("failed: %w", err)
	}
	return nil
}

after

id, err = "<Create>"
if err != nil {
	return xerrors.Errorf("failed: %w", err)
}

xxx, err = "<特定の処理A>"
  if err != nil {
	return xerrors.Errorf("failed: %w", err)
}

if ("条件に一致した時、処理を終わる") {
	return nil
}

まとめ

安易に、//nolint:せずにゆっくり考えてみることは大切なことを学びました。

他にも考えればもっと良い実装が思いつくのだろうとは思いますが、Lintがあるおかげで少なくとも読みやすいコードにはなったように思います。

実装力ついた（かな。）

おわり

GoのLinterさん【calculated cyclomatic complexity】は自分のコード力が低いってこと？