LoginSignup
27
8
@kobayashimakoto(誠 小林)inダイレクト出版株式会社

【ruby】 nil を渡さない 返さない

Posted at

はじめに

最近、コードレビューで「なるほど~⭐️」と思うことがあったので備忘録としてまとめようと思いました。
それは、プリミティブな nil を返却するとそれ自体が意味を持ち始め、コードの意図が伝わりにくくなるというものでした。

nilを返したくない理由

  • ロジックの至る所でnilを意識した設計になり、コードを読む時や機能追加する時に nilが返却されるかどうかの丁寧な確認が必要 になるのでだるい

  • nil が返されることで条件分岐が発生し、可読性の低いコードを書いてしまいがち。

  • 「nilが返却される」 というのが一種の暗黙的な仕様になる。

nil を返すことで引き起こされる悲劇

では悲劇が起こる前...

タスクと1週間のタスクを管理するようなクラスがあるとします。

#
# タスククラス
# 何曜日に実行されないといけないか知っている
#
class Task
  TODO = 'todo'
  DOING = 'doing'
  DONE = 'done'

  attr_reader :day, :title

  def initialize(title, day)
    @title = title
    @day = day
    @status = TODO
  end

  def doing
    @status = DOING
  end

  def done
    @status = DONE
  end

  def done?
    @status == DONE
  end
end

1週間分のタスクを管理するクラス

#
# 週間タスククラス
#
class WeeklyTask
  def initialize(task_list)
    @weekly_tasks = {
      monday: nil,
      tuesday: nil,
      wednesday: nil,
      thursday: nil,
      friday: nil,
      saturday: nil,
      sunday: nil
    }

    task_list.each do |task|
      @weekly_tasks[task.day] = task
    end
  end

  def done_task(day)
    @weekly_tasks[day].done
  end

  # 指定した曜日のタスクを出力
  def day_task(day)
    @weekly_tasks[day].title
  end

  # タスクの進捗出力
  def progress
    done_count = @weekly_tasks.values.count { |task| task.done? }
    task_count = @weekly_tasks.length
    return "0%" if task_count.zero?
    
    percent = (done_count.to_f / task_count * 100).round
    "#{percent}%"
  end
end

この時の自分はあまりにストイックで1週間の全てに自己研鑽の予定を入れていました。
これでも一応は動きます。

task_list = [
  Task.new('rubyの勉強', :monday),
  Task.new('ベンチプレス', :tuesday),
  Task.new('vue.jsの勉強', :wednesday),
  Task.new('スクワット', :thursday),
  Task.new('デザインパターンの勉強', :friday),
  Task.new('懸垂', :saturday),
  Task.new('クリーンアーキテクチャの勉強', :sunday)
]

weekly_task = WeeklyTask.new(task_list)

puts weekly_task.day_task(:thursday)
# => スクワット

weekly_task.done_task(:monday)
weekly_task.done_task(:thursday)
puts weekly_task.progress
# => 29%

ところが、ベンチプレスの筋肉痛があまりにひどくて筋トレは1週間に一回しかできないことが後でわかり、急遽予定を削除しました。

その結果どうなるでしょうか。

task_list = [
  Task.new('rubyの勉強', :monday),
  Task.new('ベンチプレス', :tuesday),
  Task.new('vue.jsの勉強', :wednesday),
  # Task.new('スクワット', :thursday),
  Task.new('デザインパターンの勉強', :friday),
  # Task.new('懸垂', :saturday),
  Task.new('クリーンアーキテクチャの勉強', :sunday)
]

  weekly_task = WeeklyTask.new(task_list)
  puts weekly_task.day_task(:thursday)
  # => undefined method `title' for nil:NilClass (NoMethodError)

title というメソッドに応答できず、NoMethodError が発生します。
日付に対しnilが指定されるケースが発生するからです。

    @weekly_tasks = {
      monday: Task,
      tuesday: Task,
      wednesday: Task,
      thursday: nil,
      friday: Task,
      saturday: nil,
      sunday: Task
    }

この前提があったとして、NoMethodErrorがraiseされない用に
WeeklyTaskの各ロジックを修正します。


class WeeklyTask
  def initialize(task_list)
    @weekly_tasks = {
      monday: nil,
      tuesday: nil,
      wednesday: nil,
      thursday: nil,
      friday: nil,
      saturday: nil,
      sunday: nil
    }

    task_list.each do |task|
      @weekly_tasks[task.day] = task
    end
  end

  def done_task(day)
    @weekly_tasks[day]&.done
  end

  # 指定した曜日のタスクを出力
  def day_task(day)
    @weekly_tasks[day]&.title
  end

  # タスクの進捗出力
  # ここは仕方なく、タスクがある日のみを母数で考える
  def progress
    task_list = @weekly_tasks.values.compact
    done_count = task_list.count { |task| task.done? }
    task_count = task_list.length
    return "0%" if task_count.zero?
    
    percent = (done_count.to_f / task_count * 100).round
    "#{percent}%"
  end
end

  task_list = [
    Task.new('rubyの勉強', :monday),
    Task.new('ベンチプレス', :tuesday),
    Task.new('vue.jsの勉強', :wednesday),
    # Task.new('スクワット', :thursday),
    Task.new('デザインパターンの勉強', :friday),
    # Task.new('懸垂', :saturday),
    Task.new('クリーンアーキテクチャの勉強', :sunday)
  ]
  
  weekly_task = WeeklyTask.new(task_list)
  puts weekly_task.day_task(:thursday)
  # => nil

&やcompactなど各メソッドの内部でnilを意識した設計になってしまいました。

しかも、day_taskは返却値がnilになることもあるので、受け取る側のロジックで nilかどうかの条件分岐が発生するでしょう。 つまりTaskが設定されていなかった場合にどのような処理が走るのかは、利用する側のコードを見ないとわからないです。

そのため、後からコードを読む時や機能追加する時もnilが返却されるかどうかを
丁寧に確認しないといけません。

特に嫌なのは、「nilが返却される」 = 「予定のない日」
というのが暗黙的なルールになってしまっています。

何もしないオブジェクト

NoMethodErrorによるトラブルや、nilチェックを避けるためにも、
nil を返さない設計が必要です。

返却される側からすると常にTaskオブジェクトが返却されるのがシンプルですね。

例えば、

    WEEK_DAYS = [:monday, :tuesday, :wednesday, :thursday, :friday, :saturday, :sunday]
  
    def initialize(task_list)
      # 1週間の予定を先に作成することでnilの管理をなくす
      @weekly_tasks = WEEK_DAYS.each_with_object({}) do |day, hash|
        hash[day] = Task.new(Task::PENDING, day)
      end

      task_list.each do |task|
        @weekly_tasks[task.day] = task
      end
    end

上記の用に「何もしないオブジェクト（未定のタスク）」を先埋めすることで、nilを管理する必要がなくなります。

その結果、WeeklyTaskクラスでもnilを意識する必要性がなくなり、
Task に plan?メソッドを名付けることで 「未定」という状態（仕様）が可視化される用になりました。

class Task
    TODO = 'todo'
    DOING = 'doing'
    DONE = 'done'
    PENDING = '未定'
  
    attr_reader :day, :title
  
    def initialize(title, day)
      @title = title
      @day = day
      @status = TODO
    end
  
    def doing
      @status = DOING
    end
  
    def done
      @status = DONE
    end
  
    def done?
      @status == DONE
    end
  
    def planned?
      @title != PENDING
    end
  end
  
  class WeeklyTask
    WEEK_DAYS = [:monday, :tuesday, :wednesday, :thursday, :friday, :saturday, :sunday]
  
    def initialize(task_list)
      @weekly_tasks = WEEK_DAYS.each_with_object({}) do |day, hash|
        hash[day] = Task.new(Task::PENDING, day)
      end
  
      task_list.each do |task|
        @weekly_tasks[task.day] = task
      end
    end
  
    def done_task(day)
      @weekly_tasks[day].done
    end
  
    # 指定した曜日のタスクを出力
    def day_task(day)
      @weekly_tasks[day].title
    end
  
    # タスクの進捗出力
    # ここは仕方なく、タスクがある日のみを母数で考える
    def progress
      task_list = @weekly_tasks.values.filter{ |task| task.planned? }
      done_count = task_list.count { |task| task.done? }
      task_count = task_list.length
      return "0%" if task_count.zero?
  
      "#{(done_count.to_f / task_count * 100).round}%"
    end
  end
  
  task_list = [
    Task.new('rubyの勉強', :monday),
    Task.new('ベンチプレス', :tuesday),
    Task.new('vue.jsの勉強', :wednesday),
    # Task.new('スクワット', :thursday),
    Task.new('デザインパターンの勉強', :friday),
    # Task.new('懸垂', :saturday),
    Task.new('クリーンアーキテクチャの勉強', :sunday)
  ]
  
  weekly_task = WeeklyTask.new(task_list)
  
  puts weekly_task.day_task(:thursday)
  # => 未定
  
  weekly_task.done_task(:monday)
  weekly_task.done_task(:thursday)
  puts weekly_task.progress
  # => 20%

利用する側は、nil が返却されることを意識しなくてよくなりました。

余談:
上記のように NULL を表現する専用オブジェクトを作ることを俗に NULLオブジェクトパターンとか呼んだりもするそうです。（rubyならnilオブジェクトパターンになるのか...?）

参考

Active RecordとNull Objectパターン

最後に

ここまで見ていただきありがとうございました。（^人^）

「nil を返さない」というのはシンプルですが、実装中は脳みそが目の前の処理に慣れているので、違和感に気づけないことが意外と多いです。
せっかく記事にしたので同じ轍を踏まないようにしていきたいですね💦

27
8
0

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
27
8