やわらかテック

業務を通して得られた知見、知見。個人的に試してみたこと、興味のあることについてアウトプットしています。

特に意味のないデフォルト引数が保守性を低下させるので注意

機能追加の依頼がきた😮

いつものように業務をこなしていると、スケジュール管理機能にとある機能追加の依頼が来ました。
内容としては複数のスケジュールを作成する関数で祝日での場合には、祝日に該当する日にはスケジュールを作成しないようにしたいとのことです。 「お、こんなのすぐ出来るやん!」と思い、修正対象の関数を見てみると以下のように定義されていました。

def create_schedules(end_date, start_date = nil)
  :
  :
end

処理の内容としては第2引数のstart_dateから第1引数のend_dateまでの日付にスケジュールを作成するというシンプルなものです。
開始日を指定する第2引数にはデフォルト引数が設定されており、start_dateに指定がない場合には当日からend_dateまでの日付にスケジュールを作成するという仕様になっていました。

def create_schedules(end_date, start_date = nil)
    start_date = start_date.nil? ? Time.zone.now : start_date
end

発生した問題🙄

このデフォルト引数が設定されていたことで結果的に実装の工数がかなり増えてしまいました。自分としてはcreate_schedulesの第3引数に祝日の場合にスケジュールを作成しないためのフラグ変数not_created_on_public_holiday(Boolean)を追加したいなと考えていました。
しかし第2引数がデフォルト引数として定義されてしまっているため、新しく引数を追加する場合には、今回追加を検討している第3引数も同様にデフォルト引数にするか、start_date = nilの前の箇所へ引数を追加する必要があります。
not_created_on_public_holidayに対してデフォルト引数を追加するメリットがない & end_datestart_dateが順に並んでいた方が使用感が良いと判断したので、今回は第1引数として新たな引数を追加して以下のように関数を定義することにしました。

def create_schedules(not_created_on_public_holiday, end_date, start_date = nil)
    start_date = start_date.nil? ? Time.zone.now.to_date : start_date
end

あとはnot_created_on_public_holidaytrue/falseに従って処理を変化させてあげればいいだけなのですが、create_schedulesを呼び出している箇所がそこそこあり、今回の引数の追加と順番の変更に多くの時間を使うことになってしまいました。
仮にcreate_schedulesが以下のように定義されていたのであれば、もっと早く修正をする事が出来ました。また祝日にスケジュールしないというデフォルト引数の設定も簡単に出来ました。

def create_schedules(start_date, end_date)
  :
end

# 祝日にデフォルトでスケジュールを作成しないver
def create_schedules(start_date, end_date, not_created_on_public_holiday = false)
  :
end

反省点✒️

呼び出し元が多かったので、not_created_on_public_holidayにデフォルト引数が適応出来れば、修正はかなり楽だったでしょう。またend_datestart_dateの順序が逆になっているのも気になります。引数1つの定義の仕方で、これだけ作業量が変わってくるのかと思い知らされました。
特に意味のないデフォルト引数の場合には可能な限り使用を避けるべきです。今回の場合はstart_dateend_dateという特定の期間に対してスケジュールを作成するという仕様は当然の仕様なのでデフォルト引数を用いる必要はありません。

仮にstart_dateを当日に固定してスケジュールを作成したいのであれば、こんな感じでラップした関数を定義してあげれば良いです。

def create_schedules_today_to_end(end_date)
  start_date = Time.zone.now.to_date
  create_schedules(start_date, end_date)
end

def create_schedules(start_date, end_date)
  :
end

デフォルト引数でやりたいことの多くは、パラメーターを固定したラップ関数を定義してあげれば表現可能です。
もしくは部分適応を用いてパラメーターを固定することでも表現可能ですが、言語との相性もあるので、どちらを使用するかは相性次第かなと考えます。
少なくとも今回、自分が担当したRubyではラップ関数を定義した方が楽ですね。

const create_schedules = (start_date) => (end_date) => {
    console.log(`${start_date} ~ ${end_date}`);
};

const fixedToday = create_schedules("2021/10/08");
fixedToday("2021/10/21"); // 2021/10/08 ~ 2021/10/21

参考

qiita.com

ifritjp.github.io