ls コマンドを作るオブジェクト指向版 の課題提出 #11
Conversation
- 一旦、最低限カレントディレクトリのファイルを返す実装とテストを追加
- bin/ls.rb
- 実行ファイル
- lib/ls_command
- 実行コマンドからpathを受け取りFIleの配列を持つ
- lib/ls_file
- Fileの名前など情報を持つクラス
- 上記のテストと、テストに必要なファイルを追加
- 必要要件であるaオプションとrオプションの実装追加 - テストを追加 - 各オプションの分岐をプライベートメソッドに切り出す
- LsFileという命名は限定的すぎるので、FileDataに変更する
- Fileクラスが大きくなりすぎるため、FileStatus情報を別クラスとして切り出す
- `-l`オプションをdisplayから分ける
- Optionを継承した以下クラスを追加
- FormatOption
- `-l`オプションに該当
- MatchOption
- `-a`オプションに該当
- SortOption
- `-r`オプションに該当
- FileStatusをシンプルに実装 - ディレクトリを移動
- 不要なメソッドを削除 - Optionを切り分けたため、分岐を削除
- rubocopで指摘された箇所をまとめて修正
- if文を三項演算子へ変更
- 実行時のターミナルの幅を取得する
- shortとlongで処理を分ける
- shortの場合はターミナルの幅と文字数で列数を決定する
- longは特定の項目だけ最大値を取得し幅を表示する
- 併せてテストを修正
- formatoptionの責務が大きいので、出力のフォーマットに関するクラスを切り出し、判断だけするようにした
- Format
- 抽象クラス
- LongFormat
- `-l`適用時のトータルブロック、ファイルステータスの情報を生成
- ShortFormat
- 通常時のファイル名一覧の情報の列と行の加工と生成
不要なrequireの記述を削除 不要なテストを削除 不要な改行を削除
- LongFormatでのみ使用するため、LsCommandクラスにあったtotal_blockをLongFormatクラスに移動する
delete: unnecessary attr_reader - 外部クラスから呼ばれないため file_name, :file_statusを削除
- 不要な改行を削除
- インスタンス変数が重複していたり、情報が重複していたためfilestatusのハッシュを作成する処理をfile_dataに持たせる - file statusには純粋にfile statusの情報を持たせるようにする
- 変数名をよりわかりやすく
- renderがメソッドを呼ぶだけのメソッドになっていたので処理を移す
- widthのテストを追加 - テストファイルを保持するフォルダの名前を変更+移動
- 不要な組み込みクラスを削除 - 使用しているクラスに移動
- 誤って削除してしまっていたため、`require 'io/console'`を追加
- 引数としてshort_format_dataを持っていなかったので持たせ、renderでjoinして表示するように修正
- pathで統一しているためpathnameになっているところを修正 - 単数のfileを扱う処理がfilesになっていたのでfileに修正
- user情報など端末によって異なるため、実際のlsを実行しその結果と比較するように修正する
| require_relative 'test_helper' | ||
| require_relative '../lib/ls_command' | ||
|
|
||
| class LsCommandTest < Minitest::Test |
|
|
||
| require_relative 'option' | ||
|
|
||
| class MatchOption < Option |
There was a problem hiding this comment.
オブジェクトは現実のモノやコト、または概念などをあらわし、それらの状態とそのオブジェクトできることの2つを記述します。 MatchOption というのはどういうものなのか名前から想像が難しく、あまりオブジェクトとして適切ではないように思いました。やっていることもファイル名のArrayをフィルタしているだけなので、オプション自体の仕事、というよりはLsCommand自体やたとえばFileFetcherクラスみたいなのがあってそちらがオプションに応じてファイル名のArrayを作ってくれる、というほうがまだ直感的かなと思います。
| require 'pathname' | ||
| require_relative 'file_status' | ||
|
|
||
| class FileData |
There was a problem hiding this comment.
こちらと FileStatus を分ける意義があまり感じられませんでした。ファイルのメタデータを扱うクラスが一つあれば十分かなと思いました。どうでしょうか。
- オブジェクトとしてオプションがファイルを操作するのは違和感があるので、クラス自体を削除しLsCommadnクラス内でパラメーターで分岐して処理を行う
- FormatクラスはFormatそのものではなく、ファイルのフォーマットを整えるクラスのためFormatterという名称に修正＆継承しているクラスも統一する - Formatterクラスでrender処理するのは想定外の動きになってしまうのでメソッド名を formatに変更
- FileDataとFileStatusは1対1なのでクラスを分ける必要がないため、FileStatusを削除しFileDataにステータス関連の情報を生成する処理を統合する
- 使用していないテスト用のテキストファイルの削除
07.ls_object/lib/format/formatter.rb
Outdated
| @files = files | ||
| end | ||
|
|
||
| def render; end |
07.ls_object/lib/ls_command.rb
Outdated
| @files = build_files | ||
| end | ||
|
|
||
| def display |
There was a problem hiding this comment.
display というと表示部分まで含まれていそうに思いますので、やや意味がぼやけますが、 formatted_output とするなど何か文字列が返ってくることがわかるような名前にするとよいかなと思いました。
There was a problem hiding this comment.
displayというと表示部分まで含まれていそうに思います
確かにputsでの表示は別のクラスでやっていますので、displayだと曖昧になってしまいますね・・！
formatted_output 、すごくしっくりきたのでこのまま使用させていただきます。
- renderをformatに修正(他クラスは修正ずみ)
- displayというメソッド名だが、描画は別クラスで行なっており、LsCommandではフォーマット済み文字列を出力するメソッドのため、formatted_outputに修正
yoshitsugu
left a comment
yoshitsugu
left a comment
There was a problem hiding this comment.
クラス構成などはよいと思います 👍
細かい機能面についてだけコメントしました。
07.ls_object/lib/file/file_data.rb
Outdated
| end | ||
|
|
||
| def format_file_type | ||
| @full_path.directory? ? 'd' : '-' |
There was a problem hiding this comment.
前回のオブジェクト指向でないlsコマンドの提出物ではこれ以外のファイルタイプや、パーミッション部分もスティッキービットなども対応されていたようなので、そちらを踏襲してもいいかなと思いました。
- 特殊権限に関する処理とテストを追加
- 様々なfiletypeに対応する実装とテストを追加
- フルパス指定だったSymbolicLinkを相対パスに修正
- フルパス指定じゃないとうまくリンクされないと思って都度シンボリックリンクを作成するようにしていたが、相対パスでシンボリックリンクを作れたので削除
07.ls_object/lib/file/file_data.rb
Outdated
|
|
||
| return permission_type if special_permission != target_special_permission | ||
|
|
||
| special_permission_type = SPECIAL_PERMISSION_TYPE[special_permission] |
There was a problem hiding this comment.
前回のlsで指摘すべき範囲ではありますが、これだとダメな場合があります。なぜかというとspecial permissionは一つだけとは限らず、SUIDかつSGIDの場合などもあります。そのときoctet表現としては 2 + 4 = 6となります。なので厳密にはビット演算で何ビット目が1になっているかどうか、を見るのがよいです。実際、手元で
-rwsr-sr-x 1 yoshitsugu yoshitsugu 0 2月 9 2023 test8.txt
と表示されるファイルを作ってみたのですが、このlsコマンドだと
-rwxr-xr-x 1 yoshitsugu yoshitsugu 0 2 9 14:29 test8.txt
のようになってしまいました。
この部分自体、あまり本筋とは関係ないので、1時間とか時間決めて調べてもらって、わからなさそうなら一旦このプラクティスとしてはOKとして余裕あるときに見てもらえればよいかなと思います。
yokomaru
force-pushed
the
my-ls-object-main
branch
2 times, most recently
from
3b91cfa to
2059488
Compare
- 特殊権限の処理を、各ビットに１が立っていたら設定するように修正 - 特殊権限を兼ねるテストの追加 - テストのユーザ名をマスクする - generateがついているメソッド名を修正 - type_and_modeメソッドをtypeとmodeに分ける
yokomaru
force-pushed
the
my-ls-object-main
branch
from
2059488 to
16634c1
Compare
ls コマンドを作るオブジェクト指向版の課題を提出いたします。
お手隙でご確認お願いいたします！
課題URL
https://bootcamp.fjord.jp/products/20766