LoginSignup
15
5

More than 1 year has passed since last update.

@tuchiro(takuto)

業務でプログラムを始める入門者のために保守エンジニアの観点から読みやすいコードの構造を考えてみた

Last updated at Posted at 2021-12-10

この記事は

10日目の記事です。

この記事の趣旨

それなりの年数システム屋をやっており、それなりの数のプロジェクトにおいてコーディングの修正やレビューを行ってきましたが、度々「読むのつらいなー」と思うコードに出会ってきました。

今後システム屋になる後輩向けに簡単なサンプルを用いた解説をしたいと思い作成しました。
色々ご意見などいただけたら今後もアップデートしたいと思います。

仮想の要件

この記事では開発初期の要件を以下と仮定してサンプルコードを実装します。
同一の要件、出力結果の２つの実装を比較してポイントを解説する形となります。

  • 仮想のECサイト、「つちろマート」は月の利用総額に応じて顧客ランクを設定し、用途不明の「つちろポイント」を特典として付与するとします。
  • ポイントには顧客ランクに応じて付与される固定ポイントと、顧客ランクと利用総額に応じた割合で付与される変動ポイントがあります。
  • 顧客ランクには利用総額に応じてランク1〜ランク３が存在します。

以下の表が顧客ランクと「つちろポイント」の要件です。

  • つちろマート ポイント制度の要件表V1
ランク１会員 ランク２会員 ランク3会員
利用総額 10000円未満 10000円 50000円
固定ポイント なし 100pt 500pt
変動ポイント率 1% 3% 10%

想定される月次の特典付与は以下のような処理になるでしょう。

  • 顧客ランク特典処理の概要設計V１
  1. 利用額から翌月の顧客ランクを算出する。
  2. 顧客ランクに応じた固定ポイントを付与する。
  3. 顧客ランクと利用額に応じた変動ポイントを付与する。

まれによくある辛いコードV1

カスタマーランクに応じたポイントに関する仕様をコードに落とします。
実際の開発だとRDBにつないだEctoのスキーマで作成するケースが多いかと思いますが、
今回はサンプルなので構造体のリストをハードコードしたデータストアのダミー実装となっております。

カスタマーランクテーブルV1
lib/tuchro_mart/cutomer_ranks.ex
defmodule TuchroMart.CustomerRanks do


  # カスタマーランクの一覧を取得する
  def getCustomerRanks() do

    [
    %{rank: "1", fix_point: 0, flex_point_rate: 0.01 },
    %{rank: "2", fix_point: 100, flex_point_rate: 0.03 },
    %{rank: "3", fix_point: 500, flex_point_rate: 0.1 },
    ]
  end

  # 指定したカスタマーランクを取得する
  def getCustomerRank(customer_rank) do
    [ current_customer_rank ] = getCustomerRanks()
    |> Enum.filter(fn(x) ->
      x[:rank] == customer_rank
    end)
    current_customer_rank
  end
end

次にビジネスロジック層となる処理フローを実装します。
「Eixirらしくない」構文が多いですが、「よく見る辛いコード」ということで書いています。
(Elixirは別に関数関数しなくても書ける懐の広い言語です！)

つらいビジネスロジックV1
lib/tuchro_mart/tsurai_buz_flow.ex
defmodule TsuchroMart.TsuraiBuzFlow do

  alias TuchroMart.CustomerRanks

  # 顧客ランク特典付与
  def give_customer_rank_benefits(total_use_amount) do

    # 現在のつちろポイント(サンプルの為ハードコード)
    benefits = %{tsuchro_point: 100}

    #　利用額から翌月の顧客ランクを算出する。
    customer_rank =
    cond do
      total_use_amount >= 50000 ->
        "3"
      total_use_amount >= 10000 ->
      "2"
      true ->
      "1"
    end

    # 顧客ランクテーブルから対象のランクのデータを取得
    rank_table = CustomerRanks.getCustomerRank(customer_rank)

    #　顧客ランク1以外なら固定ポイントを付与する
    benefits =
    if customer_rank != "1" do
      benefits
      |> Map.put(:fix_point, rank_table.fix_point)
      |> Map.put(:tsuchro_point, benefits.tsuchro_point + rank_table.fix_point)
    else
      benefits
      |> Map.put(:fix_point, 0)
    end

    # 顧客ランクと利用総額に応じた変動ポイントを付与する。
    flex_point = total_use_amount * rank_table.flex_point_rate
    benefits = benefits
      |> Map.put(:flex_point, flex_point)
      |> Map.put(:tsuchro_point, benefits.tsuchro_point + flex_point )

  end

end

この関数の実行結果は以下のようになります

iex(1)> TsuraiBuzFlow.give_customer_rank_benefits(100)
%{fix_point: 0, flex_point: 1.0, tsuchro_point: 101.0}
iex(2)> TsuraiBuzFlow.give_customer_rank_benefits(10000)
%{fix_point: 100, flex_point: 300.0, tsuchro_point: 500.0}
iex(3)> TsuraiBuzFlow.give_customer_rank_benefits(50000)
%{fix_point: 500, flex_point: 5.0e3, tsuchro_point: 5.6e3}

これを読んで「なんだこれ？」と思う方はたくさんいると思いますが、現実の案件で保守を引き継ぐと「また君かー」というような頻度で度々こんな感じのコードに出逢います。
(そうじゃない方は多分幸せで、ご自身もしっかりした方なのでこの記事の続きは読まなくても大丈夫です)

細かく解説するまでもなく問題点は明確です。

  1. ビジネスロジック本体に細かい処理を記述している為、全体像を大まかに把握したい段階でも最初から最後まで読む必要があります。
  2. 初期要件の条件表をそのままifやcase、condで条件分岐しているため、分岐も複雑です。上の仕様表が失われている場合(要件定義書が存在しないシステムというのもまれによくある)仕様のバリエーションの全体像を把握することも困難になります。

そしてやってくる要件の追加

現実のシステム開発では当然初期の開発で終わりということはなく、
仕様の変更、追加があり、その度に要件は複雑になることが多いです。

ここで次の要件追加があったと仮定しましょう。

  • ランク１会員にも１０ptを付与する
  • 既存の利用額に応じた会員ランク以外に年会費利用のプレミアム会員を追加する
  • ランクに応じてつちろポイント以外の特典を付与する

その場合の要件が追加された顧客ランク表は以下となります。

ランク１会員 ランク２会員 ランク3会員 プレミアム会員
利用総額 10000円未満 10000円 50000円 不問
固定ポイント 10pt 100pt 500pt 500pt
変動ポイント率 1% 3% 10% 10%
その他特典 なし なし 豪華粗品抽選券を発送 毎月豪華粗品を発送

まれによくある辛いコードV2

要件を追加した顧客ランクテーブルが以下となります。

カスタマーランクテーブルV2
lib/tuchro_mart/cutomer_ranks_v2.ex
defmodule TuchroMart.CustomerRanksV2 do


  # カスタマーランクの一覧を取得する
  def getCustomerRanks() do

    [
    %{rank: "1", fix_point: 10, flex_point_rate: 0.01 },
    %{rank: "2", fix_point: 100, flex_point_rate: 0.03 },
    %{rank: "3", fix_point: 500, flex_point_rate: 0.1 },
    %{rank: "p", fix_point: 500, flex_point_rate: 0.1 },
    ]
  end
~ 以下略 ~

追加要件を反映したビジネスロジックが以下になります。

つらいビジネスロジックV２
lib/tuchro_mart/tsurai_buz_flow_v2.ex
defmodule TsuchroMart.TsuraiBuzFlowV2 do

  alias TuchroMart.CustomerRanksV2

  # 顧客ランク特典付与
  def give_customer_rank_benefits(total_use_amount, is_premium) do

    # 現在のつちろポイント(サンプルの為ハードコード)
    benefits = %{tsuchro_point: 100}

    #　利用額から翌月の顧客ランクを算出する。
    customer_rank =
    cond do
      is_premium -> # V2追加 プレミア会員
        "p"
      total_use_amount >= 50000 ->
        "3"
      total_use_amount >= 10000 ->
      "2"
      true ->
      "1"
    end

    # 顧客ランクテーブルから対象のランクのデータを取得
    rank_table = CustomerRanksV2.getCustomerRank(customer_rank)

    #　V2変更 顧客ランクに応じて固定ポイントを付与する
    benefits = benefits
      |> Map.put(:fix_point, rank_table.fix_point)
      |> Map.put(:tsuchro_point, benefits.tsuchro_point + rank_table.fix_point)

    # 顧客ランクと利用総額に応じた変動ポイントを付与する。
    flex_point = total_use_amount * rank_table.flex_point_rate
    benefits = benefits
      |> Map.put(:flex_point, flex_point)
      |> Map.put(:tsuchro_point, benefits.tsuchro_point + flex_point )

    # V2追加 顧客ランクに応じて特別な特典を付与する(サンプルの為、文字列の要素を追加するのみ)
    benefits =
    cond do
      customer_rank == "p" ->
        benefits
        |> Map.put(:special_benifit, "豪華粗品をお送りします！")
      customer_rank == "3" ->
        benefits
        |> Map.put(:special_benifit, "豪華粗品抽選券をお送りします！")
      true ->
        benefits
    end

  end

end

この関数の実行結果は以下のようになります

iex(1)> TsuraiBuzFlowV2.give_customer_rank_benefits(100, false)
%{fix_point: 10, flex_point: 1.0, tsuchro_point: 111.0}
iex(2)> TsuraiBuzFlowV2.give_customer_rank_benefits(10000, false)
%{fix_point: 100, flex_point: 300.0, tsuchro_point: 500.0}
iex(3)> TsuraiBuzFlowV2.give_customer_rank_benefits(50000, false)
%{
  fix_point: 500,
  flex_point: 5.0e3,
  special_benifit: "豪華粗品抽選券をお送りします！",
  tsuchro_point: 5.6e3
}
iex(4)> TsuraiBuzFlowV2.give_customer_rank_benefits(100, true)   
%{
  fix_point: 500,
  flex_point: 10.0,
  special_benifit: "豪華粗品をお送りします！",
  tsuchro_point: 610.0
}

元の条件分岐にさらに枝を追加することで、ビジネスロジック層全体がさらに長くなり、分岐がさらに複雑になりました。
今回は簡単なサンプルですので「まだまだヨユーヨユー」という方もいるかと思いますが、現実にはここで足し算したりMap.Put()したりしているところが、やたらとバリエーションのある税率計算だったり、複利の利息計算だったり、顧客の業務毎に必要なより複雑な処理だったりするわけです。
そうやって一目見るだけで「そっ閉じ」したくなるコードが出来上がっていきます。

個人的にこうして欲しいコード

上記と比較して私が個人的にこうあって欲しいと思っているコードが以下になります。
(カスタマーランクテーブルは差異なし)

個人的にこうして欲しいビジネスロジックV1

lib/tuchro_mart/iine_buz_flow.ex
defmodule TsuchroMart.IineBuzFlow do

  alias TuchroMart.CustomerRanks

  # 顧客ランク特典付与
  def give_customer_rank_benefits(total_use_amount) do

    # 現在のつちろポイント(サンプルの為ハードコード)
    benefits = %{tsuchro_point: 100}

    #　利用額から翌月の顧客ランクを算出する。
    customer_rank = calc_customer_rank(total_use_amount)

    # 顧客ランクテーブルから対象のランクのデータを取得
    rank_table = CustomerRanks.getCustomerRank(customer_rank)

    #　固定ポイントを付与する(顧客ランク1の固定ポイントは0)
    benefits = give_fix_point(benefits, rank_table)

    # 顧客ランクと利用総額に応じた変動ポイントを付与する。
    benefits = give_flex_point(benefits, total_use_amount, rank_table)

  end

  # 利用総額に応じて顧客ランクを算出する
  defp calc_customer_rank(total_use_amount) do
    customer_rank =
      cond do
        total_use_amount >= 50000 ->
          "3"
        total_use_amount >= 10000 ->
          "2"
        true ->
          "1"
      end
  end

  # 固定ポイントを付与する(顧客ランク1の固定ポイントは0)
  defp give_fix_point(benefits, rank_table) do
    benefits = benefits
      |> Map.put(:fix_point, rank_table.fix_point)
      |> Map.put(:tsuchro_point, benefits.tsuchro_point + rank_table.fix_point)
  end

  # 顧客ランクと利用総額に応じた変動ポイントを付与する。
  defp give_flex_point(benefits, total_use_amount, rank_table) do
    flex_point = total_use_amount * rank_table.flex_point_rate
    benefits = benefits
      |> Map.put(:flex_point, flex_point)
      |> Map.put(:tsuchro_point, benefits.tsuchro_point + flex_point )
  end

end

ポイントを説明すると

固定ポイントの付与がないケースを0ポイントの付与に読み替える。
 #　固定ポイントを付与する(顧客ランク1の固定ポイントは0)
    benefits = give_fix_point(benefits, rank_table)

これにより不要なIF条件がなくなります。
「XXの場合は、XXする(またはしない)」といった仕様があると、反射的にIF分を書く人がいますが、本当にその分岐が必要か今一度己に問うてみてください。

ビジネスロジック本体は概要設計のレベルに抽象化し、具体的な処理は別関数で処理分割する

defで定義したビジネスロジックから詳細な処理をdefpで定義したプライベート関数に分離しています。
これにより一度全体像の把握をした上で、次により具体的、詳細な実装を読むことができ、コードからより容易に処理内容を把握できるようになります。
個人的にはよく整理されたコードと要件定義、外部仕様(画面設計、IF設計、DB設計など処理記述以外の設計)があれば詳細設計書は不要と考えています。
逆にコードが整理されていない場合、修正や影響範囲の把握、詳細設計書のメンテナンスに工数がかかり、しかも「本当に詳細設計書があっているか分からないので詳細設計書を確認して、毎回コードも確認する」といったことになります。
修正作業が必要な分、設計書の存在が保守のお荷物になることになります。

最初に提示した

  • 顧客ランク特典処理の概要設計V１
  1. 利用額から翌月の顧客ランクを算出する。
  2. 顧客ランクに応じた固定ポイントを付与する。
  3. 顧客ランクと利用額に応じた変動ポイントを付与する。

と上記のビジネスロジックはほぼ＝で翻訳できるレベルですので、個人的にはこのレベルであれば詳細設計書は不要だと思います。

個人的にこうして欲しいビジネスロジックV2

前項のコードに「まれによくある辛いコード」と同じく改修を加えます。

lib/tuchro_mart/iine_buz_flow_v2.ex
defmodule TsuchroMart.IineBuzFlowV2 do

  alias TuchroMart.CustomerRanksV2

  # 顧客ランク特典付与
  def give_customer_rank_benefits(total_use_amount, is_premium) do

    # 現在のつちろポイント(サンプルの為ハードコード)
    benefits = %{tsuchro_point: 100}

    #　利用額から翌月の顧客ランクを算出する。
    customer_rank = calc_customer_rank(total_use_amount, is_premium)

    # 顧客ランクテーブルから対象のランクのデータを取得
    rank_table = CustomerRanksV2.getCustomerRank(customer_rank)

    #　固定ポイントを付与する
    benefits = give_fix_point(benefits, rank_table)

    # 顧客ランクと利用総額に応じた変動ポイントを付与する。
    benefits = give_flex_point(benefits, total_use_amount, rank_table)

    # V2追加 顧客ランクに応じて特別な特典を付与する(サンプルの為、文字列の要素を追加するのみ)
    benefits = give_special_benefits(benefits, customer_rank)

  end

  # 利用総額に応じて顧客ランクを算出する
  defp calc_customer_rank(total_use_amount, is_premium) do
    customer_rank =
      cond do
        is_premium -> # V2追加 プレミア会員
          "p"
        total_use_amount >= 50000 ->
          "3"
        total_use_amount >= 10000 ->
        　　　"2"
        true ->
        　　　"1"
      end
  end

  # 固定ポイントを付与する(顧客ランク1の固定ポイントは0)
  defp give_fix_point(benefits, rank_table) do
    benefits = benefits
      |> Map.put(:fix_point, rank_table.fix_point)
      |> Map.put(:tsuchro_point, benefits.tsuchro_point + rank_table.fix_point)
  end

  # 顧客ランクと利用総額に応じた変動ポイントを付与する。
  defp give_flex_point(benefits, base_tsuchro_point, rank_table) do
    flex_point = base_tsuchro_point * rank_table.flex_point_rate
    benefits = benefits
      |> Map.put(:flex_point, flex_point)
      |> Map.put(:tsuchro_point, benefits.tsuchro_point + flex_point )
  end

  # V2追加 顧客ランクに応じて特別な特典を付与する(プレミアム会員)
  defp give_special_benefits(benefits, "p") do
    benefits
      |> Map.put(:special_benifit, "豪華粗品をお送りします！")
  end

  # V2追加 顧客ランクに応じて特別な特典を付与する(ランク３会員)
  defp give_special_benefits(benefits, "3") do
    benefits
      |> Map.put(:special_benifit, "豪華粗品抽選券をお送りします！")
  end

  # V2追加 顧客ランクに応じて特別な特典を付与する(その他顧客ランクは特典なし)
  defp give_special_benefits(benefits, customer_rank) do
    benefits
  end

end

以下にポイントを解説します。

データのバリエーションに落とし込めばコードの修正がいらない
 #　固定ポイントを付与する
    benefits = give_fix_point(benefits, rank_table)

最初から顧客ランク1の固定ポイントは0ポイントの加算としておいたことで、要件変更によるビジネスロジックの修正が必要ありません。

データのバリエーションに落とし込めない仕様はいっそパターン毎大胆に分岐させる
 # V2追加 顧客ランクに応じて特別な特典を付与する(サンプルの為、文字列の要素を追加するのみ)
    benefits = give_special_benefits(benefits, customer_rank)

追加要件のその他特典について。
サンプルの為、文字列の属性追加のみ行っていますが、要は「テーブルに落とし込んで汎用化しづらい仕様」だと思ってください。
その場合、処理ステップ毎に複雑なIF条件で分岐するより、パターン事にごっそり別ロジックとして分岐した方がよほど理解しやすくなります。

第二引数のcustomer_rankの値によってプレミアム会員 "p" なら

defp give_special_benefits(benefits, "p")

が、ランク３会員 "3" なら

defp give_special_benefits(benefits, "3")

その他であれば

defp give_special_benefits(benefits, customer_rank)

が実行されます。

特定のランクの処理内容が変更された場合、該当の関数のみを修正すれば他のランクに影響を与えないことが断言できますし(無影響確認テストをしなくて良いとは言っていない)、「特別な特典が必要なランク」が追加された場合はランクテーブルと関数を追加すれば,ビジネスロジック層を修正することなく改修できます。

パターン毎に分岐した処理の先で共通して同じような処理をする部分があるならコピーではなく、さらに共通関数として切り出します。
その場合、その処理は「現状結果が同じである」だけなのか「本質的に同じ処理である」なのかは注意が必要です。

このサンプルではElixirのパターンマッチで実装していますが、オブジェクト指向言語などでも、多態性の実装を用いて同じようなことができます。

Elixirでは私が以前投稿した記事BehaviourとMix.Configで切り替え可能なStubを実装するのようにIF分を書かずに処理を変更する実装は他にも存在します。
IF分以外に適切な選択肢がないか、常に心に止めおくべきかと思います。

パイプは最高だぜ！

最後に、処理フロー部分をElixirのパイプを用いて記述するとここまでスッキリ書けるという例です。

lib/tuchro_mart/cool_buz_flow_v2.ex
defmodule TsuchroMart.CoolBuzFlowV2 do

  alias TuchroMart.CustomerRanksV2

  # 顧客ランク特典付与
  def give_customer_rank_benefits(total_use_amount, is_premium) do

    # 現在のつちろポイント(サンプルの為ハードコード)
    benefits = %{tsuchro_point: 100}

    #　利用額から翌月の顧客ランクを算出する。
    customer_rank = calc_customer_rank(total_use_amount, is_premium)

    # 顧客ランクテーブルから対象のランクのデータを取得
    rank_table = CustomerRanksV2.getCustomerRank(customer_rank)

    benefits = benefits
    |> give_fix_point(rank_table) #　固定ポイントを付与する(顧客ランク1の固定ポイントは0)
    |> give_flex_point(total_use_amount, rank_table) # 顧客ランクと利用総額に応じた変動ポイントを付与する。
    |> give_special_benefits(customer_rank) # 顧客ランクに応じて特別な特典を付与する(サンプルの為、文字列の要素を追加するのみ)

  end

~ 以下略 ~

パイプは連続して関数を呼び出す場合

こんなのが
# function1,2,3の順で実行したい
x = function3(function2(function1(y)))

の「処理の順序と記述の順序が逆転する」という問題を

こうなる
x = y
|> function1()
|> function2()
|> function3()

解決してくれる素敵な言語仕様なので、Elixirでビジネスロジックを書くなら是非とも活用してほしい構文です。

まとめ

あくまで個人的な経験に基づく意見のため、他にも色々な意見やより良い現場での工夫や、
「ランク判定もっとやり方あるだろ」とか色々ご指摘あるかと思いますが、
ここまで最近レビューをしながら考えていただことをアウトプットしてみました。
まとめると以下となります。

  • 実装は1日~数週間でも保守は1年や5年メンテナンスし続けることがざら、書きやすさやコードの短さ至上主義ではなく、読みやすさ、メンテナンスしやすさを重視してほしい(希望)
  • if文、case文以外の選択肢はないか今一度考えて欲しい(切望)
  • 単純な条件の分岐はパターンマッチで読みやすくできる。そうElixirならね！
  • あとパイプは最高！
15
5
2

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
Sign upLogin
15
5