Kadai1,2 kotaaaa #87
Kadai1,2 kotaaaa #87
Conversation
kadai1/kotaaaa/search/search.go
Outdated
| // Get the files in the target directory | ||
| files, err := ioutil.ReadDir(dir) | ||
| if err != nil { | ||
| log.Fatal(err) |
There was a problem hiding this comment.
エラーはここで処理するのではなく、この関数を
func GetFiles(dir string, ext string) ([]string,error) {
とした上で呼び出し元で処理した方が良いです。理由としてはそのようにエラーハンドリングするのがGoの一般的な書き方であるほか、以下のような問題があるためです
- エラーのハンドリングを呼び出し元でコントロールできない
- この関数をCLI以外で使うときに中断されると困るケースがある
- ログを出したくないときに使いづらい
- log.Fatal(err)は内部でos.Exit(1)を呼び出すため、main()関数以外で使用すると他のゴルーチンの終了を待たずして大元であるmain()ゴルーチンを終了させてしまう
kadai1/kotaaaa/search/search.go
Outdated
| name := file.Name() | ||
| // If the file is directory, add files recursively. | ||
| if file.IsDir() { | ||
| for _, subFile := range GetFiles(dir+name, ext) { |
There was a problem hiding this comment.
パスの結合はfilepath.Joinを使う方が良いでしょう。ディレクトリの区切りが/以外のプラットフォームでも対処できますし、ディレクトの最後に/がつくつかないで誤動作することがなくなります
kadai1/kotaaaa/search/search_test.go
Outdated
| for _, ext := range extentions { | ||
| files := GetFiles("../testdata/", ext) | ||
| for _, file := range files { | ||
| if filepath.Ext(file) != ext { |
There was a problem hiding this comment.
ここではGetFilesが意図どおりに動くかのテストをすべきかと思うので、拡張子が想定通りかを調べるよりファイル名が想定通りかを調べるほうが望ましいかと思います
kadai1/kotaaaa/search/search.go
Outdated
| log.Fatal(err) | ||
| } | ||
|
|
||
| var arr []string |
There was a problem hiding this comment.
細かいところになりますが、[]stringはarrayではなくsliceです([3]stringはarrayです)。変数名はarrではなく、slやlist,意味ある名前にするならtargetFilesとかresultsにするのが適切かと思います。変数名をどれにすべきかは場合によるのですが、とりあえずarrayは適切でなさそうとことを覚えておいてもらえると良さそうです
| } | ||
| if !validateFileFormat(targetSrcExt) || !validateFileFormat(targetDstExt) { | ||
| return errors.New("Error: Invalid or Unsupported file format") | ||
| } |
There was a problem hiding this comment.
入力と出力でどちらが問題なのかわかるように分けておいた方が親切かと思います
| }{ | ||
| {"Success1", "../testdata", ".png", ".jpg", false}, | ||
| {"Success2", "../testdata", ".jpg", ".gif", false}, | ||
| {"Success2", "../testdata", ".gif", ".png", false}, |
There was a problem hiding this comment.
名前が同じだとどちらがエラーとなったときにわかりづらいほか、1,2というように番号を振ってくと途中に追加するときに煩雑だったりどの番号がどのケースかがわかりづらいのでcaseNameを以下のようにわかりやすくつけとくと良いかと思います。
png to jpg
jpg to gif
jpg to svg
|
kadai1/kotaaaa/converterにビルド済みのファイルがあげらていますが、これは環境毎に異なるものになりますし、ファイルサイズも大きいのでgitに含めない方が良さそうです |
|
@gosagawa さん |
課題1を作成しました。
使い方については、READMEに記載しています。
お時間あれば、レビューをよろしくお願いします。
課題1
2021/11/06追記
課題2 の単体テストの追加