wcコマンドを作成 by s-tone-gs · Pull Request #10 · s-tone-gs/ruby-practices

s-tone-gs · 2025-09-10T04:39:09Z

レビューお願いいたします

JunichiIto

コメントしました！

あと、実行結果のスクショを見ましたが、オリジナルのwcコマンドは数値が右寄せになっているので、ここもできたら見た目を合わせてほしいです〜。

JunichiIto · 2025-09-15T23:11:06Z

05.wc/wc.rb

+  display(totals, flags)
+end
+
+get_option_and_paths(ARGV) => { line:, word:, byte:, paths: }


82-100行目はmainメソッドに入れてみてください〜。
https://qiita.com/jnchito/items/4b4cae54170cc2f4377e

コードリーディングをしやすくするために修正にて反映しました！

JunichiIto · 2025-09-15T23:14:31Z

05.wc/wc.rb

+else
+  paths.each do |path|
+    outputs = {}
+    if File.directory?(path)


ディレクトリのケースって、スクショ載せてます？
https://bootcamp.fjord.jp/pages/write-code-write-test

載せていなかったので新しく追加しました

JunichiIto · 2025-09-15T23:15:33Z

05.wc/wc.rb

+  {
+    line: File.read(path).lines.count,
+    word: File.read(path).split.count,
+    byte: File.size(path),


byte: input.sizeの結果と数値が変わりそうですね。
同じ入力値に対しては同じ出力値が出るようにしてほしいです。

出力する値を間違えていたので修正にて反映しました

JunichiIto · 2025-09-15T23:16:06Z

05.wc/wc.rb

+def build_file_output(path)
+  {
+    line: File.read(path).lines.count,
+    word: File.read(path).split.count,


読み取り対象のファイルが巨大な場合、readを繰り返すとメモリの消費や実行時間が増えてしまいそうです。

実行効率を上げるために修正にて修正しました！

JunichiIto · 2025-09-15T23:16:57Z

05.wc/wc.rb

+    byte: File.size(path),
+    name: path
+  }
+rescue Errno::ENOTDIR


このあたりのエラー処理も実装したらエビデンスを残しておきたいですね。

提出物の中にスクショを新しく追加しました！

JunichiIto · 2025-09-15T23:18:14Z

05.wc/wc.rb

+
+def display(outputs, flags)
+  # ループ処理にするとflagsとoutputsの要素の順番が同期する必要があるため、今後予期せぬバグに繋がると判断しこのように記述している。
+  outputs.delete(:line) unless flags[:line]


引数を破壊的に変更するのは避けたいですね。
https://bootcamp.fjord.jp/pages/do-not-use-destructive-method

今後予期せぬ副作用が起こらないように修正にて反映しました

JunichiIto · 2025-09-15T23:19:10Z

05.wc/wc.rb

+def calculate_total(totals, outputs)
+  return totals if outputs.key?(:error)
+
+  totals[:line] += outputs[:line]


ここも引数の内容を破壊的に変更しています

今後予期せぬ副作用が起こらないように修正にて反映しました

JunichiIto

動画でレビューしたので別途リンクを送ります〜。

s-tone-gs · 2025-09-22T01:45:55Z

05.wc/wc.rb

-  return str_output.rjust(width) if key == :line || :word || :byte
-
-  str_output.ljust(width)
+def calculate_output_width(text_metadata_collection)


入力に応じて動的に幅を計算するメソッドを追加し、後ほどのadjust_styleと組み合わせて右揃えのレイアウトを実現しました

s-tone-gs · 2025-09-22T01:48:48Z

05.wc/wc.rb

-    end
-    display_totals(totals, output_flags) if paths.length > 1
-  end
+  option_and_paths => { paths:, **option_flags }


パターンマッチの結果を分割代入し、output_flags = { line:, word:, byte: }で行われていた無駄な再定義を無くしました

s-tone-gs · 2025-09-22T01:51:23Z

05.wc/wc.rb

-    byte: input.size
-  }
+def display(paths, option_flags)
+  text_metadata_collection =


元のstdins、outputsにあたるもの命名をtext_metadata_collectionとしました

s-tone-gs · 2025-09-22T01:53:41Z

05.wc/wc.rb


-def build_directory_output(path)
+def set_text_metadata(option_flags, message: nil, input: '', name: nil)
  {


build_stdin_outputとbuild_file_outputの処理をset_text_metadataにて共通化しました

s-tone-gs · 2025-09-22T01:54:18Z

05.wc/wc.rb

-  }
+def build_text_metadata(paths, option_flags)
+  text_metadata_collection = paths.map do |path|
+    if File.directory?(path)


例外処理で行っていたエラー処理をif文での実行に変えました

s-tone-gs · 2025-09-22T01:58:28Z

05.wc/wc.rb

  opt.on('-w') { |v| word = v }
  opt.on('-c') { |v| byte = v }
-  paths = opt.parse(arguments)
+  paths = opt.parse(ARGV)


ARGVを直接記述しました

s-tone-gs · 2025-09-22T02:02:29Z

05.wc/wc.rb

+      temporary[key] = (temporary[key] || 0) + data if %i[line_count word_count size].include?(key)
+    end
  end
+  totals.merge(name: 'total')


破壊的変更が起こらないように修正しました

s-tone-gs · 2025-09-22T02:02:54Z

05.wc/wc.rb

+    end
+  end
+  if text_metadata_collection.length > 1
+    text_metadata_collection + [calculate_total(text_metadata_collection)]


破壊的変更を起こさないためにこのような記述になっています。

JunichiIto

今回も動画でレビューしたので別途リンクを送ります〜。

JunichiIto

コメントしました！

JunichiIto · 2025-09-24T22:43:57Z

05.wc/wc.rb

+    if paths.empty?
+      [create_text_metadata(input: $stdin.read)]
+    else
+      build_text_metadata(paths)


create_text_metadataとbuild_text_metadata、名前だけ見ると、どちらも同じ機能を持つメソッドに見えますね。（createとbuildという動詞に大きな違いがない＆目的語もtext_metadataで同じ）

実際は提供する機能（取得されるデータ）は異なるはずなので、もっと違いがわかる名前を付けたいなと思いました。

命名を修正にて修正しました！

wcのWikipediaを参考にしました
https://en.wikipedia.org/wiki/Wc_(Unix)

JunichiIto · 2025-09-24T22:45:05Z

05.wc/wc.rb

 def main
-  option_and_paths => { paths:, **option_flags }
-  display(paths, option_flags)
+  paths, target_options = options_and_paths


options_and_pathsなら、その順番に合わせて

Suggested change

paths, target_options = options_and_paths

target_options, paths = options_and_paths

にする方がいいかも。

あと、target_は省略してもいいんじゃないでしょうか。

https://bootcamp.fjord.jp/pages/avoid-too-specific-var-name

記事ありがとうございます！
返り値や引数の順番を修正にて修正しました！

JunichiIto · 2025-09-24T22:46:47Z

05.wc/wc.rb

+    end
+  width = calculate_output_width(text_metadata_collection, target_options)
+  text_metadata_collection.each do |text_metadata|
+    render(target_options, text_metadata, width)


Suggested change

render(target_options, text_metadata, width)

render(text_metadata, target_options, width)

text_metadataをrenderするのが主目的なので、text_metadataが最初の引数にある方がいいですね。
オプションの類いは引数の順番としては後ろの方に来ることが多いです。

引数の順番にも意味があるのですね！ありがとうございます！
返り値や引数の順番を修正にて修正しました！

JunichiIto · 2025-09-24T22:55:18Z

05.wc/wc.rb

+      { line_count: line, word_count: word, size: size }.map do |option, flag|
+        option if flag
+      end.compact


Suggested change

{ line_count: line, word_count: word, size: size }.map do |option, flag|

option if flag

end.compact

{ line_count: line, word_count: word, size: size }.select { |_, v| v }.keys

こんな書き方もできます

select、keysのメソッドは初めて使いました！短く記述できて好みでした( ´∀｀ )
冗長であったため修正にて反映しました！

JunichiIto · 2025-09-24T22:56:07Z

05.wc/wc.rb

+  opt.on('-w') { |v| word = v }
+  opt.on('-c') { |v| size = v }
+  paths = opt.parse(ARGV)
+  target_options =


データの中身を考えるとオプションというより、出力対象の項目名、っていう感じがしますね 🤔

命名を修正にて修正しました！

wcのWikipediaを参考にしました
https://en.wikipedia.org/wiki/Wc_(Unix)

JunichiIto · 2025-09-24T22:57:28Z

05.wc/wc.rb

+  [paths, target_options]
+end
+
+def create_text_metadata(input:, name: nil)


Suggested change

def create_text_metadata(input:, name: nil)

def create_text_metadata(input, name = nil)

キーワード引数じゃなくてもいいんじゃないかなー、と思います
https://bootcamp.fjord.jp/pages/dont-abuse-keyword-args

キーワード引数である必要がなくなったので修正にて反映しました！

JunichiIto · 2025-09-24T22:58:42Z

05.wc/wc.rb

+  if text_metadata_collection.length > 1
+    text_metadata_collection + [calculate_total(text_metadata_collection)]
+  else
+    text_metadata_collection
+  end


Suggested change

if text_metadata_collection.length > 1

text_metadata_collection + [calculate_total(text_metadata_collection)]

else

text_metadata_collection

end

text_metadata_collection << calculate_total(text_metadata_collection) if paths.size > 1

text_metadata_collection

好みの問題ですが、こんなふうに書くのもありです

提示いただいた方が簡潔で好みでした！冗長であったため修正にて反映しました！

JunichiIto · 2025-09-24T23:02:40Z

05.wc/wc.rb

+  target_options.each do |option|
+    print "#{text_metadata[option].to_s.rjust(width)} "
+  end
+  print text_metadata[:name]
+  puts ''


Suggested change

target_options.each do |option|

print "#{text_metadata[option].to_s.rjust(width)} "

end

print text_metadata[:name]

puts ''

values = target_options.map do |option|

text_metadata[option].to_s.rjust(width)

end

puts [*values, text_metadata[:name]].join(' ')

みたいにするとちょっとスッキリするかも

冗長であったため修正にて反映しました！

JunichiIto

いくつかコメントしましたが大きな問題ではないのでこれでOKです！🙆‍♂️

JunichiIto · 2025-09-25T23:37:19Z

05.wc/wc.rb

+  opt.on('-w') { |v| word = v }
+  opt.on('-c') { |v| size = v }
+  paths = opt.parse(ARGV)
+  column_names = [line, word, size].none? ? %i[line_count word_count size] : { line_count: line, word_count: word, size: size }.select { |_, v| v }.keys


ちょっと横に長すぎる気がしますね。改行した方が読みやすいかも。

Suggested change

column_names = [line, word, size].none? ? %i[line_count word_count size] : { line_count: line, word_count: word, size: size }.select { |_, v| v }.keys

column_names = [line, word, size].none? ?

%i[line_count word_count size] :

{ line_count: line, word_count: word, size: size }.select { |_, v| v }.keys

JunichiIto · 2025-09-25T23:39:30Z

05.wc/wc.rb

+end
+
+def render(statistics, column_names, width)
+  filterd_statistics = column_names.map do |column_name|


filterd -> filtered ですね。
ただ、filteredだと不要なデータを取り除くイメージです。
https://docs.ruby-lang.org/ja/latest/method/Array/i/filter.html

ここでやってるのは見た目の調整だと思うので、formatted とかの方が良さそうです

wcコマンドを作成（l、w、cオプション）

65c31b5

s-tone-gs force-pushed the wc branch from c1156d2 to 65c31b5 Compare September 11, 2025 00:46

s-tone-gs added 2 commits September 11, 2025 10:12

rubocopで修正

f16378f

flagsによる制御を最小限にするために修正

5fd8ba8

JunichiIto reviewed Sep 15, 2025

View reviewed changes

s-tone-gs added 6 commits September 16, 2025 09:32

コードリーディングをしやすくするために修正

581b8fc

実行効率を上げるために修正

1ee4ccd

今後予期せぬ副作用が起こらないように修正

77ea30e

出力のスタイルを修正

44beccb

出力する値を間違えていたので修正

c0ac463

rubocopの修正を反映

0eed6c8

JunichiIto reviewed Sep 16, 2025

View reviewed changes

s-tone-gs added 2 commits September 22, 2025 10:34

出力幅が動的に変化するように修正

90934e9

上から下に自然に読めるように修正

ea6443d

s-tone-gs commented Sep 22, 2025

View reviewed changes

JunichiIto reviewed Sep 23, 2025

View reviewed changes

s-tone-gs added 10 commits September 24, 2025 10:02

メソッドからの返り値の受け取り方を変更

3d09423

メソッド設計を見直し

0506db2

余計な改行を削除

07680e4

メソッド名を見直し

f38807f

必須要件外の実装を削除

c0dd417

合計算出のメソッドのロジックをシンプルに修正

18cd3cf

レイアウト調整のメソッドをシンプルに

e1fceb9

オプションの出力制御の方法を変更

bc05889

rubocopの修正提案を反映

740c96f

変数名を修正

1ac3773

JunichiIto reviewed Sep 24, 2025

View reviewed changes

s-tone-gs added 3 commits September 25, 2025 10:20

返り値や引数の順番を修正

d31e9b1

命名を修正

e11ccf9

キーワード引数である必要がなくなったので修正

fa413ca

冗長であったため修正

200be15

s-tone-gs force-pushed the wc branch from 3a3ca3e to 200be15 Compare September 25, 2025 03:05

JunichiIto approved these changes Sep 25, 2025

View reviewed changes

	paths, target_options = options_and_paths
	target_options, paths = options_and_paths

	render(target_options, text_metadata, width)
	render(text_metadata, target_options, width)

	def create_text_metadata(input:, name: nil)
	def create_text_metadata(input, name = nil)

Conversation

s-tone-gs commented Sep 10, 2025

Uh oh!

JunichiIto left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JunichiIto left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JunichiIto left a comment

Choose a reason for hiding this comment

Uh oh!

JunichiIto left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

s-tone-gs Sep 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

s-tone-gs Sep 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

s-tone-gs Sep 25, 2025 •

edited

Loading

s-tone-gs Sep 25, 2025 •

edited

Loading