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

機能追加の依頼がきた😮

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

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_dateとstart_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_holidayのtrue/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_dateとstart_dateの順序が逆になっているのも気になります。引数1つの定義の仕方で、これだけ作業量が変わってくるのかと思い知らされました。
特に意味のないデフォルト引数の場合には可能な限り使用を避けるべきです。今回の場合はstart_dateとend_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

参考

リファクタリング 既存のコードを安全に改善する（第2版）

qiita.com

ifritjp.github.io

動的型付け言語(Ruby)でも関数の戻り値の型は可能な限り統一した方が良い

出会いは突然に😲

いつものように眠い目を擦りながらコードレビューをしていると以下のようなコードに遭遇しました。
(※部分的に書き換えてあるので、実際には動作していないコードです)

def fetch_companies(setting_id)
  setting = Setting.find_by(id: setting_id)
  if setting
    companies = Company.where(setting.kind)
    companies
  end
end

処理としては指定されたidのSettingデータを取得して、Settingに記録されているkindを持つCompanyの一覧を取得するという単純なものです。ActiveRecord(O/RMapper)を使ったデータベースとのやりとりを行う処理です。
動作自体には何の問題もないのですが、この関数は指定したidを持つ、Settingの値が見つかった時と見つからなかった時で返す値の型が異なります。

RubyやElixirでは明示的にreturn構文を書かなくても最終行に記述されている値が自動で返ります。現時点では以下のように値が返ります。

• Settingが見つかった時 -> Companyの配列 もしくは空配列
  • whereは1件もヒットしなかった場合に空配列を返します
• Settingが見つからなかった時 -> nil

def verification
end

res = verification
p res # nil

この関数に型アノテーションをしてあげるとすると[Company] | nilという感じになります。

何が問題なのか🤔

状態によって返る値の型が異なることが分かりましたが、これがどのような問題につながるでしょうか。
オブジェクト指向をサポートする言語でよくある例としてはfetch_companiesの戻り値に対してmapやfilterを適応したい場合です。

resp_companies = fetch_companies(6)
extract_names = resp_companies.map { |company| company.name }

このコードはSettingが存在していた場合にCompanyの配列が返ってきてくれた場合には上手く動作してくれますが、Settingが見つからなかった場合にはどうでしょうか。先ほど記述したように、その場合にはnilが返ってきます。nilにはmap関数は実装されていないので、当然ですがnil::NilClassエラーになります。

nil.map { |company| company.name }
# undefined method `map' for nil:NilClass (NoMethodError)

この問題を回避するために、fetch_companiesの呼び出し元で戻り値を精査する必要があります。

resp_companies = fetch_companies(6)
return [] if resp_companies.empty? 
extract_names = resp_companies.map { |company| company.name }

fetch_companiesを呼び出す箇所が少なければ良いのでしょうが、たくさんあったとするならば、この単調な判定を各所に記述するのは面倒です。

解決策📖

解決方法はシンプルで静的型付け言語で当たり前のように行う、関数の戻り値の型を統一してあげれば良いです。
今回の場合では空配列を返すのが適切です。これでSettingの有無に関わらず、全ての場合で空配列か、Companyの配列が返るようになります。

# fetch_companies :: number -> [Company] 
def fetch_companies(setting_id)
  setting = Setting.find_by(id: setting_id)
  return [] if setting.nil?

  Company.where(setting.kind)
end

心なしかifのネストも無くなりコードの見栄えも良くなりました。必ず配列が返るので先程の問題は発生しません。 合わせて、呼び出し元でnilが返ってくる場合を考えなくて良くなりました。やりました。関数の戻り値の型を揃えるのは良いことだらけです🎉

xn--97-273ae6a4irb6e2hsoiozc2g4b8082p.com

入門Haskellプログラミング

プログラミングの基礎 ((Computer Science Library))

ファシリテーターをまかせたら普通にみんなやってくれた

リモートワークのファシリってつらい

現在、弊社ではコロナ対策として、完全リモートワークに移行しており、よほどのことがない限りは在宅にて勤務をしております。開発業務がメインなのですが、スクラムマスターという立場上、ファシリテーターとして主体的に話す機会や、プロダクトオーナーや外部の方とMTGをする機会がそこそこあったりします。

特に毎日の朝会(デイリースクラム)では、ファシリテーターとして現在のタスクの進捗状況を確認する必要があります。定刻になり、「朝会やります〜」の一言から一日の仕事が始まります。

リモートワークに移行して早一年、この環境にも慣れてきたせいか、チームメンバーの反応が薄くなってきたなぁと感じています。 移行当初は問題が起きれば、zoomで声をかけて確認、相談といったことが当たり前でしたが、気づけばslackを使ったコミュニケーションに移行しました。これ自体は効率化を図るためであり、無駄な時間を双方使う必要がないので、素晴らしいことです。しかしながら、この温度感を朝会のような場に持ってこられるとファシリテーターとしてはつらいところがあります。

👨「進捗はどうでしょうか？」
👨‍👨‍👦‍👦「...（シーン)」
👨「何か困っていることはありますか？」
👨‍👨‍👦‍👦「...（シーン)」
👨「不明な点はありますか？」
👨‍👨‍👦‍👦「...（シーン)」

🥺

と様々な問いかけをするのですが、無反応になってしまうor特定のメンバーだけが受け答えをするという状況になってしまいました。 反応がない以上、こちら側としてはOKなのか分からないですし、返答を待つ間(返答がないかもしれませんが)の沈黙はかなりつらいものがあります。

外部メンターの方に相談してみた

上記の発生している問題を外部メンターの方に相談してみました。頂いたアドバイスを要約しました。

人が何かを決めてくれるというのは非常に楽。今、発言をする必要があるという認識をしてもらう必要がある。また、発言がしやすい雰囲気であることも大切。そのためにファシリテーターとして踏み込みすぎていないか(内容を決めてyes/noで聞いていないか)を注意してみてください。

確かに自分は先に全てを説明して、内容を決めた後に「どうでしょうか」と聞いてしまう、まさに踏み込みすぎていました。この点に関しては個人的な反省点として気をつけていかなければなりません。雰囲気作りに関しても、自分の腕の見せ所ですぐに出来るようになるものでもないでしょう。

合わせて心理的安全性の確保についてのお話も頂きました。4つの不安をどのように取り除くかが重要とのことでした。

不安①：無知だと思われることへの不安が増す
不安②：無能だと思われることへの不安が増す
不安③：邪魔をしていると思われることへの不安が増す
不安④：ネガティブだと思われることへの不安が増す

www.dodadsj.com

難しいのは今、発言しなければならないという認識をしてもらうことです。意見を求めていることを理解してもらうためにはどうすればいいのかと相談した結果、「メンバーにもファシリテーターをやってもらえばいいんじゃないかな」とアドバイスを頂きました。

ファシリテーターをやってみることで普段、OKBさんが何を求めているのか、発言してほしいのか、反応がほしいのか、提案してほしいのかが体験を通じて理解していくことが出来ると思います。大切なのはファシリテーターの視点になってみることです。人前で話す練習にもなります。

ということでファシリテーターをまかせてみた

スプリントごとの振り返り会で上記の「みんなの反応がなくて困っている」という話を正直にしてみました。一瞬、空気が重くなってしまい「やってしまったなぁ...」と思ったのですが、メンバーから「話し出していいのか分からなかった」という声が続々と上がり始めて、皆、同じようなことで悩んでいたんだなぁと気付かされました。合わせて、メンバーにファシリテーターを任せてみたいという話をしました。

出来るかどうか分からないという不安の声が多かったですが、とりあえずやってみろの精神で1人1回ずつはやってもらうことにしました。

...。

驚いたのですが、皆、普通にファシリテーターをやれていました。「進捗どうですか？」「不明点ありますか？」と自分からメンバーに確認する姿はファシリテーターそのものでした。何を不安がっていたのか正直、驚きです。また、メンバーの各times(slackの個人チャネルのtwitterのようなもの)では、

• ファシリって難しい
• 反応ないとこんな感じなのか
• 普通に出来た

など、様々な声が上がっていました。

note.com

ファシリテーターを体験してもらうことで、メンバーの視野が広がりました。結果的には朝会で問題なければ「問題なしです」という声が積極的にあがるようになり、ファシリテーターをする側としては非常に楽になりました。出来ないかもと決めつけず、お願いしてみるという事が非常に重要だと勉強になりました。

SCRUM BOOT CAMP THE BOOK【増補改訂版】 スクラムチームではじめるアジャイル開発

(これのリモートワーク版出ないかなぁ...)