nilを含むソートの表示が上手くいかない。「undefined method `sort_by' for nil:NilClass」

Question

前提・実現したいこと

LINEのようなメッセージ機能を持つ「トークルーム一覧」において、メッセージが作成された順で各トークルームを降順に並べ替える実装を施しました。

しかし、メッセージの授受がまだ行われていないトークルームをユーザーが保持している場合、そのユーザーのトークルーム画面はエラー表示になると言う問題が起こっています。

空のトークルームデータを削除し、メッセージのやり取りのあるトークルームのみを残すと、エラーメッセージもなくページが正常に表示されます。

該当のソースコード

Rooms_controller.rb

  def set_another_entries
    # ログイン中のユーザーの持つEntryを配列にする。
    my_room_ids = @current_user.entries.pluck(:room_id)
    # 上で定義した「ログイン中のユーザーの持つEntry」の内、「@current_userと異なるuser_id」、つまり自分以外の相手一覧を表示する。
    @another_entries = Entry.includes(:user, :room).where(room_id: my_room_ids).where('entries.user_id != ?', @current_user.id)    
    
    # エントリーを配列にし、各エントリーを「メッセージが作成された順」で並べ替える。reverseでエントリーを降順にする。
    @entries = @another_entries.to_a.compact!.sort_by {|entry| entry.messages.order(created_at: :desc).first&.created_at}.reverse
  end

NoMethodError in RoomsController#index
undefined method `sort_by' for nil:NilClass
Extracted source (around line #67):

66
67    @entries = @another_entries.to_a.compact!.sort_by {|entry| entry.messages.order(created_at: :desc).first&.created_at}.reverse
68

※Entryとは Userと Roomを繋ぐ中間テーブルで、これをビューにて繰り返し表示しています。

ここに至るまでの過程

ここに至るまでにも何度かエラーが出てきましたので、どのように対策したかを書いていきます。

①最初、該当のコードは以下の記述でした。

@entries = @another_entries.to_a.sort_by {|entry| entry.messages.order(created_at: :desc).first.created_at}.reverse

NoMethodError in RoomsController#index
undefined method `created_at' for nil:NilClass
Extracted source (around line #67):

66 
67     @entries = @another_entries.to_a.sort_by {|entry| entry.messages.order(created_at: :desc).first.created_at}.reverse
68

②created_at がnilと言う事だったので、その前に&.を付けました。

@entries = @another_entries.to_a.sort_by {|entry| entry.messages.order(created_at: :desc).first&.created_at}.reverse

ArgumentError in RoomsController#index
comparison of NilClass with ActiveSupport::TimeWithZone failed
Extracted source (around line #67):

66
67     @entries = @another_entries.to_a.sort_by {|entry| entry.messages.order(created_at: :desc).first&.created_at}.reverse
68

③配列に格納されている要素の中で「nil」がある事が原因(?)と思ったので、これを取り除く compact メソッドを使いました。

@entries = @another_entries.to_a.compact!.sort_by {|entry| entry.messages.order(created_at: :desc).first.created_at}.reverse

NoMethodError in RoomsController#index
undefined method `sort_by' for nil:NilClass
Extracted source (around line #67):

66
67    @entries = @another_entries.to_a.compact!.sort_by {|entry| entry.messages.order(created_at: :desc).first&.created_at}.reverse
68

ここで冒頭にも記述致しましたエラー文が表示され、対策が分からず足踏みしている状況です。
どなたかご助言頂けますと有難いです。

補足情報（FW/ツールのバージョンなど）

ruby 2.6.4p104
RubyGems 3.0.3
Rails 5.2.3

Answer 1

末尾に!をつける破壊的メソッドの多くは変更が行われない場合にnilを返しますので
メソッドチェインの中に入れる場合に注意が必要です。

compact!のリファレンス

Comments

[punchan36]

有難うございました！
メソッドに関する理解が深まりました。

Answer 2

エラーの直接の原因は回答が付いている通り、compactとcompact!の使い間違いです。

③配列に格納されている要素の中で「nil」がある事が原因(?)と思ったので、

思ったのに確かめもせずにプログラム修正したのが間違いですね。状況を混乱させる選択です。

to_aの結果にnilが含まれていたら、entry.messagesの段階でentryがnilになるので、undefined method 'messages' for nil:NilClassのエラーになるはずですよね。

nilなのはfirstの結果で、その原因はorder結果が空リストつまりmessage結果が空なのでしょう。

messageがないentryをどうしたいのでしょうか？

もしそのentryを除外したいのなら、sort_byの前でrejectやselectで除外すればいいでしょう。
もし除外せずに、最新メッセージの日時以外のキー(今現在とか)を与えたいなら、sort_byのブロック中でifでそういう処理にすればいい。
あるいは、「entryには必ずmessageがあるはず」ということなら、もっと前の段階で間違ってそうでないデータが出来てしまっています。

②created_at がnilと言う事だったので、その前に&.を付けました。

こういう延髄反射みたいな修正は駄目ですね。コーディングレベルのミスじゃなくて設計ミスなので。

Comments

[punchan36]

有難うございます！
仰る通り一つ一つの修正の仕方が間違っておりました…。
以下のコードで希望の実装が出来ました。

@entries = @another_entries.includes(:messages).to_a
.reject {|entry| entry.messages.blank?}
.sort_by {|entry| entry.messages.order(created_at: :desc).first.created_at}
.reverse

メッセージデータがまだない Entry は非表示にしたかったので、reject で対応致しました。
有難うございました！

Answer 3

エラーメッセージは、「undefined method `sort_by' for nil:NilClass」なので、質問のプログラムで、.sort_byメソッドが使わている箇所を探してみると

@entries = @another_entries.to_a.compact!.sort_by

のように、compact! に続いて、sort_byが使われているコードがあります。

compact!は、配列から nil である要素を削除した配列を返すという機能と、配列に nil が含まれていない場合には nil を返すという(余計な)機能がありますから、.compact! の前の配列に nil を含まないものが存在していることがエラーの原因だと推測されます。

[対処方法]、.compact!した結果が nil かどうかを判断して、
・nilだった場合は .compact!する前の配列を .sort_by する。
・nilでなかったら、その配列を .sort_byする。
というようにプログラムを修正してください。そうすれば問題が解決すると思います。

Comments

[winterboum]

compact! でなく 　compact にしてもらったら？

[punchan36]

有難うございます！
compact! 前の配列が nil かどうか調べた所、以下の様になりました。

[8] pry(main)> @another_entries.to_a.compact!.nil?
=> true
配列に nil が含まれていない為、nilを返したと言う事だと思います。

ただその場合の「.compact!する前の配列を .sort_by する」の部分で、.compact!をどこに置けば良いのか分かりませんでした…。申し訳ありません。
@another_entries.to_a.sort_by {|entry| entry.messages.order(created_at: :desc).first&.created_at}.reverse
これは違いますよね…。

ちなみに「!」を付けずに nil 判定を行うと false でした。
[9] pry(main)> @another_entries.to_a.compact.nil?
=> false
[10] pry(main)> @another_entries.to_a.nil?
=> false

[punchan36]

無事解決致しました。有難うございました！
compact! の使い方も間違っていましたが、それ以前の部分でつまずいておりました。

以下のコードで希望の実装が出来ました！
@entries = @another_entries.includes(:messages).to_a
.reject {|entry| entry.messages.blank?}
.sort_by {|entry| entry.messages.order(created_at: :desc).first.created_at}
.reverse