🔖

「Refactoring a Function in Elixir」のblog postを読んでみよう

2020/12/16に公開

Elixir

tech

この記事は、Elixir Advent Calendar 2020 16日目です。前日は、@ringo156さんの「ElixirでTwitterのbotを作る」でした！

今日はElixirのブログを漁ってる際に発見した、DockYardから出されていた「Refactoring a Function in Elixir」というブログ記事が勉強になったので、内容について触れていこうと思います。

何が書かれている？

「Designing Elixir Systems with OTP」という書籍のPart.1に書かれている設計の原則に倣い、簡単なゲームクイズのリファクタリング例がまとめてあります。

（よく見かけるんですよねーこの本。英語だし積ん読になるだろうなと思いつつ、この記事を書きながらポチりました😇）

以下の基本原則に従ってリファクタリングが進められていきます。

Build functions at a single level of abstraction
- SLAとかSLAPと呼ばれるものですね
Make decisions in function heads where possible
- 「条件分岐を関数ヘッドで表現しましょう」と読めば良さそう？
- 言い換えると、関数パターンマッチで表現しましょうとも解釈できそうです
- 関数ヘッドという用語については、こちらの記事がわかりやすかったです
Name concepts with functions
- 「コンセプト、概念に関数で名前づけしていきましょう」という感じかな
- 名前重要にあたる部分ですね
Shape functions for composition
- 「合成することを踏まえて関数を書こう」「小さく関数を書いて、pipeで組み立て可能な状態で書いていくべし」と解釈しています
Build single-purpose functions
- 「ひとつのことだけを行う関数を書きましょう」と解釈
- ついデカい関数になりがちですが大事

お題

マスコット当てゲーム？（海外では有名なのかな？自分が知らないだけかも）のロジックがテーマ
正解したらユーザーのスコア処理をして次の質問に進む。不正解ならerrorとする

といった感じでしょうか。以下元コードです。

# game.ex

def answer_question(%Game{current_question: current_question} = game, guess, user_name) do
    case(current_question.mascot == guess) do
      true ->
        game =
          game
          |> increase_score_for_user(user_name)
          |> check_for_winning_user(user_name)
          |> Map.put(:current_question, get_random_question())

        {:ok, game}

      _ ->
        {:error, :incorrect}
    end
  end

step1: Functions should be at single layer of abstraction

まずはSLAPにしたがって修正が入ります。

case(current_question.mascot == guess) do - We’re making a decision at the guess level to determine if the response from a user is correct

game = game |> increase_score_for_user(user_name) |> check_for_winning_user(user_name) - Game level operations occur as we a advance our game token.

|> Map.put(:current_question, get_random_question()) - Elixir datatypes level abstraction

case文の条件分岐
Gameに対する処理（≒ビジネスロジック)
Elixirのデータ構造(map)に対する処理

と粒度がバラバラになっているので、まずはここを綺麗にしていきます。

最初に、Map.putで書いている部分をpipeで繋がれている別関数と粒度を揃えるため、名前づけをして関数に切り出しています。

修正後がこちら。Map.put というElixirのデータ構造に対する素朴な処理で書いてしまっていたものが、select_question/1関数として用意されました。これにより、increase_score_for_userして、check_for_winning_userして、select_questionして... という具合にlevelが揃っています。

# game.ex

def answer_question(%Game{current_question: current_question} = game, guess, user_name) do
    case(current_question.mascot == guess) do
      true ->
        game =
          game
          |> increase_score_for_user(user_name)
          |> check_for_winning_user(user_name)
          |> select_question() # add select_question() to the pipeline

        {:ok, game}

      _ ->
        {:error, :incorrect}
    end
end

defp select_question(game) do
  Map.put(game, :current_question, get_random_question())
end

続いて、 case(current_question.mascot == guess) do の部分を修正していきます。これも「ユーザーの回答が正しいか？」というロジックが関数化されずに記述されているので、修正の余地があります。

「ユーザーの回答」をまとめるResponse構造体を用意し、そこにcorrectのkeyを生やします。correctにはcorrect?/2の結果が入るようにします。

# response.ex

defmodule Response do
  defstruct ~w[guess game user_name correct]a

  def new(game, guess, user_name) do
    %__MODULE__{
      guess: guess,
      user_name: user_name,
      game: game,
      correct: correct?(game, guess)
    }
  end

  defp correct?(%Game{current_question: current_question} = game, guess) do
    current_question.mascot == guess
  end
end

この構造体を使ってgameのロジックを書き換えています。

# game.ex

def answer_question(%Game{current_question: current_question} = game, guess, user_name) do
    response = Response.new(game, guess, user_name)
    case(response.correct) do
      true ->
        game =
          game
          |> increase_score_for_user(user_name)
          |> check_for_winning_user(user_name)
          |> select_question() # add select_question() to the pipeline

        {:ok, game}

      _ ->
        {:error, :incorrect}
    end
end

このコードの方が「最初に正解・不正解の判定をしている」というのがパッとわかるようになっています。正解不正解の条件分岐後、increase_score_for_user→check_for_winning_user→select_questionとロジックを通るのが一目瞭然になりました。

step2: Make decisions in function heads where possible

可能ならば条件分岐を関数ヘッドで記述しようという方針でコードを書き換えています。

サクッと結論のコードが書かれています。同名の関数を3つ記述し、第2引数のパターンマッチ

guess（string)
%Response{correct: true}
%Response{correct: false}

で分岐させています。

# game.ex

def answer_question(%Game{current_question: current_question} = game, guess, user_name)
    when is_binary(guess) do
  response = Response.new(game, guess, user_name)
  answer_question(game, response, user_name)
end

def answer_question(game, %Response{correct: true} = response, user_name) do
  game =
    game
    |> increase_score_for_user(user_name)
    |> check_for_winning_user(user_name)
    |> select_question()

  {:ok, game}
end

def answer_question(_game, %Response{correct: false} = response, _user_name) do
  {:error, :incorrect}
end

コードはスッキリしますね。ただ、個人的には元のcaseを使ったコードも好きですね。業務システムなどで分岐の条件自体に強い関心がある場合は、元のcaseの方がコードを読んだ時にパッと追いやすいと思います。

まとめ

原則がおさらいできる良いブログ記事でした🎉

粒度を意識し、かつ適切な命名をした関数を書いていくのは心がけたいですね。ついEnumやMapをサクッと使ってしまって、「あれ？このリスト処理は何がしたいんだ？」というのもつい陥りがちです（Elixirによらずどの言語でもそうなのですけどね）

原理原則を守ってテスト・保守がしやすく、可読性の高いコードを書いていきたいものです🚀

何が書かれている？

お題

step1: Functions should be at single layer of abstraction

step2: Make decisions in function heads where possible

まとめ

Discussion