オブジェクト指向Lsコマンドプログラム作成

[matuaya]

No description provided.

[betachelsea]

うまく役割分けとメソッド化ができているとおもいました！ 👍
少し気になったところだけコメントしました〜 ✍️

[betachelsea]

[nits] total_line_count数ぶんの要素数を持つ配列を作って中身を入れる、という感じですね。
こういう場合はRubyだと N.times.map {} がよく利用されます。また、if files[index] で 中身nilのものの除外をしているようですが、これは最後に .compact を利用するとシンプルに書けそうだなと思いました 💡

Array#compact (Ruby 3.3 リファレンスマニュアル)

# 参考例
N = 10
datas = N.times.map { |n| n % 3 == 0 ? nil : n }
pp datas #=> [nil, 1, 2, nil, 4, 5, nil, 7, 8, nil]
pp datas.compact #=> [1, 2, 4, 5, 7, 8]

[matuaya]

Array.newの使用をやめ、N.time.mapを使いファイルのグループ化を行いました。

[betachelsea]

[nits] maxes という単語は実際には無さそうなので、ちょっと違和感が大きかったです 🤔
直接 max_lengths から呼び出しても問題なさそうです。何回も max_lengths の処理が呼ばれることのコスト感が気になるのであれば、メモ化処理をするのは一手かもです。
「Ruby メモ化」で検索すると、色んな解説は出てくると思います。

# たとえばこんな感じ
def max_lengths
  @max_lengths ||= {
    size: @files.map { |file| file.size.to_s.length }.max,
    nlink: @files.map { |file| file.link_num.to_s.length }.max,
    user: @files.map { |file| file.user.length }.max,
    group: @files.map { |file| file.group.length }.max,
  }
end

( ただ、メモ化は最近だと、やりすぎるとパフォーマンスが悪化するという指摘もあったりするようです。 うーん、さじ加減次第ですかね。)

Ruby: 私がメモ化を暗黙で使わない3つの理由（翻訳）｜TechRacho by BPS株式会社
Ruby: メモ化のイディオムが現代のRubyパフォーマンスに与える影響（翻訳）｜TechRacho by BPS株式会社

[matuaya]

メモ化の情報ありがとうございました！たいへん勉強になりました。
今回の処理はメモ化を使わず直接max_lengthsから呼び出す変更にしました。

[betachelsea]

[IMO] ほとんどの呼び出しが file の内容に寄っているので、file のクラス (FileInfo) にメソッドを生やすのも一手かなと思いました 👀 ただ、maxes の内容も欲しいので、その情報の受け渡しのほうが煩雑だからこっちのほうがいい、という意見もありそうです。好みだと思うので、一意見として共有まで！ 🙏

[matuaya]

ファイルの詳細情報をまとめるメソッドをFileInfoクラスに移動しました。

[matuaya]

継承について質問があります。
現在DetailedFormatクラスがBasicFormatクラスを継承する形になっていますが、

  def initialize(file_paths)
    @files = file_paths.map { |file_path| FileInfo.new(file_path) }
  end

この部分が重複するからという理由だけで継承する形にしました。もしかしたら不要な継承だと指摘を受けるのかもしれないと思いながらこのような形にしたので、このアプローチが適切か理解があやふやです。
教えていただけると嬉しいです！

[betachelsea]

DetailedFormatクラスがBasicFormatクラスを継承する形になっていますが

おお、改めて見ると、確かに BasicFormat で定義されている中身はあんまり DetailedFormat で活用されてはないのですね。すみません、あんまり継承については着目してチェックできておりませんでした！ その観点からチェックさせてもらうと、BasicFormat は Basic というよりも 横並びのFormatのためのクラスだなと感じます。
しかし、Format という役割を (現)BasicFormat, DetailedFormat どちらにも持たせたいという気持ちもわかります。

自分がこの辺りを再検討するとしたら、インターフェースとしての BasicFormat を作り、それをもとに 横並び用のFormat ( 例えば… うーん ShortFormat とか? )と DetailedFormat をそれぞれ定義する、というアプローチをとるかもしれません。

# 動作確認はしてないので動かなかったらすみません、雰囲気だけ伝われば！

class BasicFormat
  def initialize(file_paths)
    @files = file_paths.map { |file_path| FileInfo.new(file_path) }
  end
  
  def format_lines
    # すべてのFormatは必ずこのメソッドを実装せよ！の気持ちを例外で示しておく。参考URL参照。
    NotImplementedError 
  end
end


class ShortFormat < BasicFormat
  def format_lines
     # …実装…
   end
   
   # …実装…
end

class DetailedFormat < BasicFormat
  def format_lines
     # …実装…
   end
   
   # …実装…
end

参考: インターフェースとは？～継承とは役割が違う～｜オブジェクト指向プログラミング(OOP)をおさらいしよう（３） - GiXo Ltd. ※RubyじゃなくてJavaでの例ですが
参考: Rubyで継承を強制するためにNotImplementedErrorを活用する - コード日進月歩

こんな感じでいかがでしょうか！ 💡