a・r・lそれぞれのオプションを統合する #9
Conversation
04.ls/ls.rb
Outdated
| files = create_file_list(option, make_absolute_path).compact.sort | ||
| files = files.reverse if option[:r] |
There was a problem hiding this comment.
読んでいて、create_file_list というからにはrオプションの処理もその中で済んでいてほしい気持ちになりました:pray:
すでにoptionまるごと渡しているのでそれを参照すれば済みますし、一度sortした戻り値に対して改めてreverseする必要もなくなるので効率的かと思います。
There was a problem hiding this comment.
変更内容
create_file_listメソッドにrオプションの処理を移動しました。
また、compactメソッドが不要だったので削除しました。
さらに、Dir.glob メソッドはデフォルトでsortしてくれることを知ったので、sortも削除しました。
コミットハッシュ
kaito0046
left a comment
kaito0046
left a comment
There was a problem hiding this comment.
変更はバッチリでした！
すみません、これまで気づけてなかった点があり、ご確認お願いします:bow:
04.ls/ls.rb
Outdated
| def create_file_list(option, absolute_path) | ||
| Dir.chdir(absolute_path) | ||
| option[:a] ? Dir.glob('*', File::FNM_DOTMATCH).map.to_a : Dir.glob('*').map.to_a | ||
| file_list = option[:a] ? Dir.glob('*', File::FNM_DOTMATCH).map.to_a : Dir.glob('*').map.to_a |
There was a problem hiding this comment.
前回までにコメントできてなくて申し訳ないのですが:bow: Dir.glob ~ に対して map to_a しているのってなぜでしょうか？ もともと Dir.glob ~ が配列を返すので不要な気がします（なにか僕が見落としているようでしたら教えてください〜:pray:）
There was a problem hiding this comment.
変更内容
map to_aメソッドを削除しました。
理由
私もなぜこう書いたのか忘れてしまいました。
コードを遡ると、aオプションの作成プラクティスの時からmap to_aメソッドを使って記述していました。
（コードコメントでwhy not`について記述することの大切さを改めて感じました。）
プラクティス「lsコマンドを作る5」の提出物のプルリクエストです。
aオプション、rオプション、lオプションを合体させたものになります。