Skip to content

Create wc#9

Open
yappu0 wants to merge 5 commits intomainfrom
create-wc
Open

Create wc#9
yappu0 wants to merge 5 commits intomainfrom
create-wc

Conversation

@yappu0
Copy link
Owner

@yappu0 yappu0 commented Feb 9, 2022

No description provided.

06.wc/wc.rb Outdated
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

== true は省略しても同じ結果になりますね。削除しておきましょう

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

06.wc/wc.rb Outdated
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if側とelse側で count_lines が重複しているので、書き方を変えることで重複を無くせないか検討をお願いします。puts で一気に出力するのではなく、 print で部分的に出力すると良さそうですね。

Copy link
Owner Author

@yappu0 yappu0 Feb 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

count_lines をif文の前に出せばいけそうです!
ありがとうございます!

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

よく見ると #{file} も重複していますね。これも共通化できそうでしょうか?

06.wc/wc.rb Outdated
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ここも行の出力部分がifとelseで重複していますね。
ファイルの情報の出力処理とコードを共通化できると修正する箇所も1箇所で済むかもしれませんね。

06.wc/wc.rb Outdated
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ここが -l オプションを考慮できていないようなので対応をお願いいたします🙏

words_count = cal_words_count(File.read(file))
bytes_count = File.size(file)

print lines_count
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

名前 行数 単語数 文字数 オプション という4つの情報を引数にとって、オプションに応じて出力するメソッドがあると、以下3箇所のコードをメソッドに集約できそうですね 👍

ぜひ検討してみてください🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments