Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Kadai1 s-shiraki #76

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Kadai1 s-shiraki #76

wants to merge 10 commits into from

Conversation

highpon
Copy link

@highpon highpon commented Aug 29, 2021

課題 1:画像変換コマンドを作ろう

お忙しいところ恐れ入りますが、もしよろしければレビューを頂けますと幸いです

画像を指定の形式に変換する
変換後の画像ファイルは元の画像と同じディレクトリに配置される

コマンドラインオプション

オプション 値の説明 デフォルト値
-from 変換前の画像形式(jpg,jpeg,pngから選択) jpg
-to 変換後の画像形式(jpg,jpeg,pngから選択) png
-dir ディレクトリの指定 ./

使い方

  • ビルド
go build -o image-convert
  • ディレクトリを指定して実行
./image-convert [...options] [directory]
./image-convert -from png -to jpg -dir d1

@@ -0,0 +1,18 @@
package args
Copy link

Choose a reason for hiding this comment

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

ここは特に主要なロジックがあるわけではないので、別パッケージに分けずにmainパッケージに含めて良さそうです。

if err != nil {
return err
}
slice := strings.Split(path, ".")
Copy link

Choose a reason for hiding this comment

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

拡張子を取るにはfilepath.Extというライブラリ及び関数があるのでそれを使った方が良さそうです。なお、このやり方だと.があるディレクトリ名と拡張子のないファイル名が組み合わさった時に誤動作するので好ましくないです。
https://play.golang.org/p/Jn50hY_Mt9B


func GetSelectedExtensionPath(fileType string, directory string) [][]string {
var retval [][]string
err := filepath.Walk(directory, func(path string, info os.FileInfo, err error) error {
Copy link

Choose a reason for hiding this comment

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

パスはディレクトリも含んでいるため、ディレクトリではないかのチェックが必要です。
例えばxxx.jpgという名前のディレクトリがあると誤動作します。

return nil
})
if err != nil {
log.Fatal(err)
Copy link

@gosagawa gosagawa Sep 5, 2021

Choose a reason for hiding this comment

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

エラーはここで処理せずにmain関数にreturnしてmain関数で処理をした方が良く、そうするのがGoでは一般的です。理由としては呼び出し元でエラーをどう処理するかを委ねられること、エラーが発生しうる関数であることを明示できることなどがあります。具体的にはこのライブラリをcli以外で扱うことになったらここでlog.Fatalで落とすのが望ましくありません。

"strings"
)

func GetSelectedExtensionPath(fileType string, directory string) [][]string {
Copy link

Choose a reason for hiding this comment

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

パスを返す関数なので[]stringで返した方が良さそうです。pathのみをappendしてその後の処理はパスを利用する関数で処理するのが良いかと思います。

}
defer fso.Close()

switch {
Copy link

Choose a reason for hiding this comment

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

以下のようにするのがシンプルかと思います。なお、元々の書き方だとtoと実際のエンコードの関数が逆になっているように思います。

switch to {
    case "jpg","jpeg":
         jpeg.Encode(fso, img, nil)
    case "png":
         png.Encode(fso, img)
    default:
        //エラー処理
}


switch {
case (from == "jpg" || from == "jpeg") && to == "png":
jpeg.Encode(fso, img, nil)
Copy link

Choose a reason for hiding this comment

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

pngもそうですが、エンコード処理はエラーになりうるもので関数もエラーを返しているのでエラー処理が必要です。

@gosagawa
Copy link

gosagawa commented Sep 5, 2021

おそらく不要である画像ファイルがコミットされているので消した方が良いです。テストデータとして画像をあげるケースもあるかと思うのですが、その時にはtestdataというディレクトリとし(go上でテストデータとして扱われます)、ファイルサイズも小さなものとすると良いでしょう(今上がっているMBを超えているファイルはでかいです)。

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