lsコマンドオブジェクト指向版の提出 by thmz337 · Pull Request #10 · thmz337/ruby-practices

thmz337 · 2024-07-06T13:57:51Z

No description provided.

kaito0046

クラス設計がそもそもうまくいっていない感じがあるので、もう一度オブジェクト指向について復習したうえで、クラス設計だけをまずはやり直していただくのがよさそうです。

kaito0046 · 2024-07-26T13:50:11Z

07.ls_object/file_stat.rb

+    @stat = [
+      file_mode_text(raw_stat),
+      raw_stat.nlink.to_s,
+      Etc.getpwuid(raw_stat.uid).name,
+      Etc.getgrgid(raw_stat.gid).name,
+      raw_stat.size.to_s,
+      raw_stat.mtime.month.to_s,
+      raw_stat.mtime.day.to_s,
+      raw_stat.mtime.strftime('%H:%M')
+    ]


https://bootcamp.fjord.jp/pages/class-like-class-and-non-class-like-class#%E3%82%AF%E3%83%A9%E3%82%B9%E5%90%8D%E3%81%A8%E5%90%8C%E5%90%8D%E3%81%AE%E5%A4%89%E6%95%B0%E3%82%92%E7%99%BB%E5%A0%B4%E3%81%95%E3%81%9B%E3%81%AA%E3%81%84 より:

クラス名と同名の変数がクラス内に登場する場合はたいてい設計や考え方をミスっている証拠なので、もう一度設計を考え直してみてください。

FileStat クラスは上記に該当すると思います（FileStat と @stat で厳密には同名ではありませんが、同じとみなせるかと）。

kaito0046 · 2024-07-26T13:58:16Z

07.ls_object/ls.rb

+
+require_relative 'file_stat'
+
+class LS


LS という概念は、オブジェクト指向版に変更する前にあったもともとの処理を押し込めるために独自に導き出されたものに見えます。
ボウリングのスコア計算プログラムをオブジェクト指向にアレンジしたときのことを思い出していただきたいです。あのプラクティスではプログラム中に登場する概念（登場人物）をうまく見出して、インスタンス同士のやり取りに上手に落とし込めていたと思います。現状、このプラクティスでは、そういった概念設定（モデリングといいます）にそもそも失敗してる感じがあるんですよね。
登場人物には（プログラム中にしか出てこない創作の概念ではなく）一般的に世に知られる具体的なモノのみを用いる、という制限を設けてみるとよいかもしれません。lsコマンドの処理には、どのような登場人物が出てくるでしょうか？

※モデリングに失敗した状態でプログラムを書いてもうまくいきませんし、具体的なプログラムを書いてからモデリングをやり直すのは修正コストが高いので、まずはクラス図か箇条書きリストの作成だけしてから再レビュー依頼をしていただくのがよいかもしれません

kaito0046 · 2024-11-12T22:32:22Z

07.ls_object/ls.rb

このファイルが持っている処理がまだ多すぎるように見えます 👀 基本的にはすべての処理は何らかのクラスに持たせることができるはずだからです。このファイルは「何らかのクラスに対して処理を一度呼び、表示すべき文字列（lsコマンドの結果として表示すべき形式になったファイル一覧）を取得する」「その文字列を puts する」というくらいが妥当です。

kaito0046 · 2024-11-12T22:37:35Z

07.ls_object/file.rb

+      @stat = [
+        file_mode_text(raw_stat),
+        raw_stat.nlink.to_s,
+        Etc.getpwuid(raw_stat.uid).name,
+        Etc.getgrgid(raw_stat.gid).name,
+        raw_stat.size.to_s,
+        raw_stat.mtime.month.to_s,
+        raw_stat.mtime.day.to_s,
+        raw_stat.mtime.strftime('%H:%M')
+      ]


LsFile という「ファイル」を模したオブジェクトなので、理想としては @stats で配列でまとめて情報が返ってくるよりも、file_mode nilnk など個別の情報をそれぞれオブジェクトのフィールドとして持っている方が、実際の「ファイル」に近いので読み手が理解しやすくなると思います。
今回の要件では結局のところ配列に展開して文字列にするだけなのでこの書き方でも十分と言えば十分なのですが、プログラム上でオブジェクトを作ってそいつに処理を持たせて扱うという趣旨から考えると、実際に存在する概念になぞらえて作ったオブジェクトならば、（やりすぎない、無理のない範囲で）実態に沿って属性を定義したほうが理解しやすいコードになります。

lsのメインの処理を、file_listに移動。

kaito0046

実行してみたら file_list.rb:33: syntax error, unexpected ')' (SyntaxError) ...atrix(max_column, long_format:).map { |m| m.join.rstrip!.con... になりました。
お手元でも同様でしょうか？

kaito0046 · 2024-11-21T23:11:48Z

07.ls_object/ls.rb

+options = OptionParser.new do |opts|
+  opts.banner = 'Usage: ./ls.rb [options]'
+
+  opts.on('-a', 'Include directory entries whose names begin with a dot (‘.’).')
+  opts.on('-r', 'Display files in reverse order.')
+  opts.on('-l', 'List files in the long format.')
+end
+
+begin
+  params = {}
+  options.parse!(ARGV, into: params)
+rescue OptionParser::ParseError => e
+  puts e.message
+  puts options.help
+  exit
+end


オプションもクラスに切り出してみるとよいかもしれません。

kaito0046 · 2024-11-21T23:12:12Z

07.ls_object/ls.rb

+max_column = params.include?(:l) ? 1 : 3
+
+file_names = params.include?(:a) ? Dir.glob('*', File::FNM_DOTMATCH) : Dir.glob('*')
+file_names.reverse! if params.include?(:r)


file_names の取得も LS::FileList に任せましょう。

thmz337 · 2024-11-23T03:21:02Z

@kaito0046

実行してみたら file_list.rb:33: syntax error, unexpected ')' (SyntaxError) ...atrix(max_column, long_format:).map { |m| m.join.rstrip!.con... になりました。お手元でも同様でしょうか？

私の方ではエラーが出ないようです。rubyのバージョンは以下の通りです。
ruby 3.2.2 (2023-03-30 revision e51014f9c0) [arm64-darwin23]

kaito0046 · 2024-11-27T10:41:41Z

@thmz337
失礼しました、SyntaxError は私の手元がおかしかったようです 🙇
しかし、 l オプションつきではやはりエラーが出てしまいます。何度もすみませんが、お手元でも同様かお教えください。

file_list.rb:30:in `block (2 levels) in current_directory_stats': undefined local variable or method `_1' for #<LS::FileList:0x000000014c868bf0> (NameError)
        from /Users/kaitofukai/Workspace/ls_normal/scripts/file_list.rb:29:in `map'
        from /Users/kaitofukai/Workspace/ls_normal/scripts/file_list.rb:29:in `with_index'
        from /Users/kaitofukai/Workspace/ls_normal/scripts/file_list.rb:29:in `block in current_directory_stats'
        from /Users/kaitofukai/Workspace/ls_normal/scripts/file_list.rb:28:in `map'
        from /Users/kaitofukai/Workspace/ls_normal/scripts/file_list.rb:28:in `with_index'
        from /Users/kaitofukai/Workspace/ls_normal/scripts/file_list.rb:28:in `current_directory_stats'
        from /Users/kaitofukai/Workspace/ls_normal/scripts/file_list.rb:47:in `matrix'
        from /Users/kaitofukai/Workspace/ls_normal/scripts/file_list.rb:39:in `show'
        from ../scripts/ls.rb:14:in `<main>'

kaito0046 · 2024-11-27T10:42:06Z

上記の点以外はよさそうに思いました。

thmz337 · 2024-11-28T14:05:38Z

@thmz337 失礼しました、SyntaxError は私の手元がおかしかったようです 🙇 しかし、 l オプションつきではやはりエラーが出てしまいます。何度もすみませんが、お手元でも同様かお教えください。

file_list.rb:30:in `block (2 levels) in current_directory_stats': undefined local variable or method `_1' for #<LS::FileList:0x000000014c868bf0> (NameError)
        from /Users/kaitofukai/Workspace/ls_normal/scripts/file_list.rb:29:in `map'
        from /Users/kaitofukai/Workspace/ls_normal/scripts/file_list.rb:29:in `with_index'
        from /Users/kaitofukai/Workspace/ls_normal/scripts/file_list.rb:29:in `block in current_directory_stats'
        from /Users/kaitofukai/Workspace/ls_normal/scripts/file_list.rb:28:in `map'
        from /Users/kaitofukai/Workspace/ls_normal/scripts/file_list.rb:28:in `with_index'
        from /Users/kaitofukai/Workspace/ls_normal/scripts/file_list.rb:28:in `current_directory_stats'
        from /Users/kaitofukai/Workspace/ls_normal/scripts/file_list.rb:47:in `matrix'
        from /Users/kaitofukai/Workspace/ls_normal/scripts/file_list.rb:39:in `show'
        from ../scripts/ls.rb:14:in `<main>'

こちら確認しましたが、私の方で-l オプションをつけて実行してもエラーは出ない状況です。

kaito0046

本当にすみません、改めて rbenv install しなおしてバージョンを揃えたら要件通りに動作しました！何度もやりとりさせてしまい申し訳ありませんでした 🙇

thmz337 added 5 commits July 6, 2024 22:56

lsコマンドオブジェクト指向版の提出

2357a02

lオプションを指定しなかった際に、file_namesが空になるバグを修正

ea17c74

ディレクトリが空であった場合の処理を追加

1ebd6b4

クラスの構成を変更

cbc5332

マジックコメントを追加。クラスの余計な行を削除

1d237b9

kaito0046 reviewed Jul 26, 2024

View reviewed changes

クラス構成の変更

0d534e7

kaito0046 reviewed Nov 12, 2024

View reviewed changes

fileクラスのstatインスタンス変数を、各属性に展開

dc35ead

lsのメインの処理を、file_listに移動。

kaito0046 reviewed Nov 21, 2024

View reviewed changes

thmz337 added 2 commits November 25, 2024 00:08

lsコマンドのオプションを別クラスに切り出し

519d16b

file名の取得等をFileListクラスに移動

bf80f6b

ファイルがひとつもない時のバグを修正

e7c98e1

kaito0046 approved these changes Nov 29, 2024

View reviewed changes


		require_relative 'file_stat'

		class LS

Conversation

thmz337 commented Jul 6, 2024

Uh oh!

kaito0046 left a comment

Choose a reason for hiding this comment

Uh oh!

kaito0046 Jul 26, 2024

Choose a reason for hiding this comment

Uh oh!

kaito0046 Jul 26, 2024

Choose a reason for hiding this comment

Uh oh!

kaito0046 Nov 12, 2024

Choose a reason for hiding this comment

Uh oh!

kaito0046 Nov 12, 2024

Choose a reason for hiding this comment

Uh oh!

kaito0046 left a comment

Choose a reason for hiding this comment

Uh oh!

kaito0046 Nov 21, 2024

Choose a reason for hiding this comment

Uh oh!

kaito0046 Nov 21, 2024

Choose a reason for hiding this comment

Uh oh!

thmz337 commented Nov 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kaito0046 commented Nov 27, 2024

Uh oh!

kaito0046 commented Nov 27, 2024

Uh oh!

thmz337 commented Nov 28, 2024

Uh oh!

kaito0046 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

thmz337 commented Nov 23, 2024 •

edited

Loading