Elixirらしい書き方へのリファクタ

Elixir製のライブラリのソースを読んでいると、
ifはほぼなく、ファンクションのパターンマッチとcaseで分岐が作成されています。
それらを参考にして、自分のコードを書きなおしてみようと思います。

今回直したコードのPR

case文の例

下記のコードでは、Regex.runの戻り値をmatchに受け、
if matchでmatchをチェックし、その後の処理に進んでいます。

これをifを使わず、caseを使用するようにします。

  def handle_cast({:handle_incoming, type = "message", send_data}, state) do
    Logger.debug "#{type} -> #{inspect send_data}"
    PlugManager.get_all
    |> Enum.each(fn({module, functions}) ->
      Enum.each(functions, fn({regex, func}) ->
        match = Regex.run(regex, send_data["text"])
        if match do
          [_text | captures] = match
          GenServer.cast(module, {:call_plugin, func, [send_data, captures]})
        end
      end)
    end)
    {:noreply, state}
  end

Regex.runをそのままcaseの対象として使用し、
[_text | captures] = matchの部分をcaseのマッチに持ってきました。
また以前は、matchがtrueだったパターンしかありませんでしたが、
nil時の処理も記述しました。

今回の主旨とは異なりますが、send_data["text"]の記述も面倒なので、
ファンクションのパターンマッチでtext変数に入れました。

  def handle_cast({:handle_incoming, type = "message",
      %{ "text" => text } = send_data}, state) do
    Logger.debug "#{type} -> #{inspect send_data}"
    PlugManager.get_all
    |> Enum.each(fn({module, functions}) ->
      Enum.each(functions, fn({regex, func}) ->
        case Regex.run(regex, text) do
          [_text | captures] ->
            GenServer.cast(module, {:call_plugin, func, [send_data, captures]})
          nil ->
            Logger.debug "nothing match #{text}"
        end
      end)
    end)
    {:noreply, state}
  end

当初の目的は達成していますが、このコードはネストが深いことと、
1つのファンクションにロジックを書きすぎているので、それもリファクタしたいと思います。

以下が、直した結果です。

• PlugManager.get_allもcaseの対象にしてみました。
• 1ファンクション、1ロジックに収めるようにしてみました。

コード量が多くないりましたが、関数単位でテストを書くため、
こちらのほうがテストが書きやすくなったのではないかと思います。

  def handle_cast({:handle_incoming, type = "message", send_data}, state) do
    Logger.debug "#{type} -> #{inspect send_data}"
    case PlugManager.get_all do
      map when map == %{} ->
        Logger.debug "Plugin not set"
      map ->
        each_module(map, send_data)
    end
    {:noreply, state}
  end

  defp each_module(module_map, send_data) when is_map(module_map) do
    Enum.each(module_map, fn({module, functions}) ->
      each_functions(module, functions, send_data)
    end)
  end

  defp each_functions(module, functions, send_data) when is_list(functions) do
    Enum.each(functions, fn({regex, func}) ->
      match_function(module, regex, func, send_data)
    end)
  end

  defp match_function(module, regex, func, %{ "text" => text } = send_data) do
    case Regex.run(regex, text) do
      [_text | captures] ->
        GenServer.cast(module, {:call_plugin, func, [send_data, captures]})
      nil ->
        Logger.debug "nothing match #{text}"
    end
  end

おわりです。

もっといい書き方あるよ！とかあったら、編集リクエストください