WCコマンド作成 #9
Open
WCコマンド作成 #9
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…臭変数(item)の修正。トータル値をハッシュで管理するように改善 レビューにて指摘いただいた箇所の修正。個人的に気になった個所の改善
配列filenameにつけた変数名を複数形へ修正。option_valuesの定義位置がおかしかったため移動。ifの条件を否定形ではなく、肯定形へ修正。ofを使用した変数名の修正。勘違いを誘発する変数名を修正。戻り値を使わないのにmapを使っていた箇所を修正
JunichiIto
reviewed
May 12, 2024
JunichiIto
left a comment
JunichiIto
left a comment
There was a problem hiding this comment.
途中までテキストでコメントしましたが、途中から動画に切り替えました。
動画レビューのURLは別途お知らせします〜。
前回のレビューにて指摘を受けた、強引な処理や、へんてこなコードを自分なりに改善。
JunichiIto
reviewed
May 23, 2024
JunichiIto
left a comment
JunichiIto
left a comment
There was a problem hiding this comment.
今回も動画でレビューしたので別途リンクを送ります〜。
（1点、動画でコメントし忘れた内容をテキストで書いてます）
…handle_multiple_filesメソッド)の改善。total値の取得方法を改善。
もともとはoptionの値がすべて同じ場合はという条件だったが、より意図が伝わりやすいすべてfalseだった場合という方法へ改善。標準入力で受け取った値をいったん変数へ入れる冗長な手順を削除し、受け取った値をそのままjoinするように修正。total値の取得方法として、文字列をくっつけてその文字数等を取得するやり方をしていたが、このやり方では、場合によって思いがけない誤差が生まれる可能性があるため、値の合計値を取得するやり方に修正。failnameのデフォルト値を必ず重複しない値('')に修正。
メソッド名変更 create_words_width -> calc_column_width (l.55~) ifの条件で改行されているものを見やすくするために、(l.51)
で変数へいったん代入。 handle_multiple_filesメソッドにて、total[option.to_s]としていたto_sを削除。トータル値をsumで算出
に変更。l8 { |value| value == false }を!valueへ修正。"
calc_column_widthにて、文字数の最大値を求めるコードを、自然と一番大きくなるtotalのバイト数を取得する方法に変更。handle_multiple_filesメソッドのtotalを求めるコードを簡略化。optionの値について、values.none?でtrueが返ってくる場合のみ要素をすべて反転するという処理に修正。
JunichiIto
approved these changes
May 31, 2024
JunichiIto
left a comment
JunichiIto
left a comment
There was a problem hiding this comment.
数点コメントしましたが大きな問題ではないのでこれでOKです！🙆♂️
Comment on lines
+43
to
+45
| options_count = options.count do |_key, value| | ||
| value == true | ||
| end |
There was a problem hiding this comment.
Suggested change
| options_count = options.count do |_key, value| | |
| value == true | |
| end | |
| options_count = options.count { |_k, v| v } |
vが真のカウントを取得すればいいので、== true はなくせるのと、これぐらいなら1行で書いても問題ないと思います
| columns << file_detail['l'].to_s.rjust(width) if options['l'] | ||
| columns << file_detail['w'].to_s.rjust(width) if options['w'] | ||
| columns << file_detail['c'].to_s.rjust(width) if options['c'] | ||
| columns << file_detail['name'] if file_detail['name'] != '' |
There was a problem hiding this comment.
Suggested change
| columns << file_detail['name'] if file_detail['name'] != '' | |
| columns << file_detail['name'] unless file_detail['name'].empty? |
と書くのもありです（英語っぽく読める）
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.