hokita / 課題１ by hokita · Pull Request #5 · gopherdojo/dojo8

hokita · 2020-07-06T15:43:10Z

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

課題内容

次の仕様を満たすコマンドを作って下さい

ディレクトリを指定する
指定したディレクトリ以下の JPG ファイルを PNG に変換（デフォルト）
ディレクトリ以下は再帰的に処理する
変換前と変換後の画像形式を指定できる（オプション）

以下を満たすように開発してください

main パッケージと分離する
自作パッケージと標準パッケージと準標準パッケージのみ使う
準標準パッケージ：golang.org/x 以下のパッケージ
ユーザ定義型を作ってみる
GoDoc を生成してみる
Go Modules を使ってみる

対応したこと

画像を変換
- 現状はjpg, pngのみ
- jpg, png以外はエラー表示
- 画像出力先は対象画像と同じディレクトリ
指定したディレクトリが無いとエラーを表示

動作

$ go build -o test_imgconv

$ ./test_imgconv -h
Usage of ./test_imgconv:
  -from string
        Conversion source extension. (default "jpg")
  -to string
        Conversion target extension. (default "png")

# testdata内のすべてのjpgファイルをpngに変換する
$ ./test_imgconv testdata
Conversion finished!

# testdata内のすべてのpngファイルをjpgに変換する
$ ./test_imgconv -from png -to jpg testdata
Conversion finished!

# ディレクトリの指定が無い場合はエラー
$ ./test_imgconv
Please specify a directory.

# 存在しないディレクトリの場合はエラー
$ ./test_imgconv non_exist_dir
Cannot find directory.

# 対応していない拡張子の場合はエラー
$ ./test_imgconv -from txt -to jpg testdata
Selected extension is not supported.

工夫したこと

png, jpg以外にも拡張子が増えそうなので、image_typeというinterfaceを作ってみた。
拡張子の微妙な違い（jpg, jpeg, JPGなど）にも対応できるようにした。

わからなかったこと、むずかしかったこと

go mod initで指定するmodule名に命名規則があるのか。
普段オブジェクト指向（その上動的型付け言語）で書いているので、それがgoらしいコードになっているのか不安。
- なんでもかんでも構造体メソッドにしたい願望がでてくる

hokita · 2020-07-07T00:38:45Z

こちらレビューお願いいたします。

tenntenn · 2020-07-09T01:00:09Z

+	"path/filepath"
+)
+
+func converterFactory(from string, to string) (*Converter, error) {


from, to stringと書けます

tenntenn · 2020-07-09T01:01:05Z

+	"path/filepath"
+)
+
+func converterFactory(from string, to string) (*Converter, error) {


型で十分伝わるので名前はnewで十分かなと思います

tenntenn · 2020-07-09T01:03:26Z

+func (conv *Converter) Execute(path string) error {
+	// ignore unrelated file
+	if !conv.fromImage.IsMatchExt(filepath.Ext(path)) {
+		return nil


これだと成功したのか失敗したのか無視されたのか分かりづらいので、エラーを返して、呼び出し元で判断してもらうのが良さそうですね。たとえば、os.IsNotExist関数とかみたいにエラーを判定する関数があると良いかも知れないです。
（この辺はエラー処理の章でやります！）

tenntenn · 2020-07-09T01:05:16Z

+
+	// output file
+	out, err := os.Create(conv.SwitchExt(path))
+	defer out.Close()


os.Createのエラー処理の後で、defer文でCloseをしてください。
Closeメソッドもエラーを返すので、書き込みの場合のみエラー処理をお願いします。
コマンドラインツールの章のスライドにやり方は書いてあるので確認してみてください！

書き込みの場合では Close がエラーを返す可能性があるんですね。
勉強になりました🙇

tenntenn · 2020-07-09T01:05:58Z

+	}
+
+	// output image
+	conv.toImage.Encode(out, img)


エンコードするのにエラーが起きることはない？

tenntenn · 2020-07-09T01:16:42Z

+		from = flag.String("from", "jpg", "Conversion source extension.")
+		to   = flag.String("to", "png", "Conversion target extension.")
+	)
+	flag.Parse()


mainパッケージ以外でflagパッケージに依存させない（明示的に*flag.FlagSetを保つ場合を除く）。
flagパッケージやos.Argsに依存すると、コマンドラインツールありきのパッケージになってしまいます。

tenntenn · 2020-07-09T01:18:34Z

+
+type JpegImage struct{}
+
+func (_ JpegImage) Encode(w io.Writer, m image.Image) error {


レシーバを参照しない場合は_も省略できます。

tenntenn · 2020-07-09T01:19:21Z

+}
+
+func (_ JpegImage) Extensions() []string {
+	return []string{".jpg", ".jpeg", ".JPG", ".JPEG"}


mapの方がいいのと、メソッドを呼び出すたびに生成する必要はないのでパッケージ変数にしても良さそうです

以下同様です。

tenntenn · 2020-07-09T01:25:14Z

+)
+
+const (
+	ExitCodeOk int = iota


iotaは値に意味がなく、区別が付けばいいときのみに使います。
そのため、0が成功、それ以外が失敗となっている終了コードに使わない方が良いでしょう。

- 引数を同じ型でまとめる - Close()のエラー処理を追加 - factoryメソッド名をnewを付与 - 拡張子をmapのパッケージ変数として宣言 - Encodeのエラー処理を追加

- 終了コードを明示的に数値を代入する - flag処理をmainパッケージへ移動

- Converter.Executeに渡されるファイルパスの拡張子が対象外の場合ErrUnmatchExtを返すようにする - 受け取る側でErrUnmatchExtかを判断し、ErrUnmatchExtなら無視する

tenntenn · 2020-07-20T02:16:40Z

+	return &Converter{fromImage, toImage}, nil
+}
+
+var ErrUnmatchExt = errors.New("ext does not match")


IsNotMatchExt関数が公開されているのでこちらは公開しないほうが良いです。

tenntenn · 2020-07-20T02:17:17Z

+
+	// file open
+	file, err := os.Open(path)
+	defer file.Close()


defer Closeはエラー処理のあと

tenntenn · 2020-07-20T02:17:25Z

+
+	// output file
+	out, err := os.Create(conv.SwitchExt(path))
+	defer func() {


defer Closeはエラー処理のあと

tenntenn · 2020-07-20T02:19:41Z

+
+type ImageType interface {
+	Encode(w io.Writer, m image.Image) error
+	IsMatchExt(ext string) bool


Hasとかぐらいでいい気もしますね。
pngImage.Has(ext)とかになると思うので。
長くてHasExtとか。

tenntenn · 2020-07-20T02:20:14Z

+type ImageType interface {
+	Encode(w io.Writer, m image.Image) error
+	IsMatchExt(ext string) bool
+	GetMainExt() string


こちらも Ext で十分かなと思います。

tenntenn · 2020-07-20T05:27:57Z

+}
+
+func selectImage(ext string) (ImageType, error) {
+	pngImage := PngImage{}


フィールドの初期化を伴わない構造体リテラル（コンポジットリテラル）での初期化は不要です。
構造体もゼロ値で初期化されるため、var pngImg PngImageのように変数を定義されば十分です。

tenntenn · 2020-07-20T05:29:41Z

+	}
+
+	err = filepath.Walk(dir,
+		func(path string, info os.FileInfo, err error) error {


引数のエラーのエラー処理。

tenntenn · 2020-07-20T05:30:26Z

+
+const QUALITY = 100
+
+type JpegImage struct{}


JPEGで十分だと思います。Goでは略語はすべて大文字か全部小文字にします。

tenntenn · 2020-07-20T05:31:21Z

+	"io"
+)
+
+const QUALITY = 100


定数であってもキャメルケースで書きます。
先頭が大文字だとパッケージ外に公開されます。

JPEGのQUALITYっていうのが分かるようにしたほうが良さそうですね。

hokita changed the title ~~create kadai1/hokita~~ hokita / 課題１ Jul 6, 2020

create kadai1/hokita

e43488f

hokita force-pushed the kadai1-hokita branch from 615e3de to e43488f Compare July 6, 2020 15:49

tenntenn added the kadai1 課題1 label Jul 9, 2020

tenntenn reviewed Jul 9, 2020

View reviewed changes

hokita added 3 commits July 11, 2020 15:50

レビュー修正

7fb551f

- 引数を同じ型でまとめる - Close()のエラー処理を追加 - factoryメソッド名をnewを付与 - 拡張子をmapのパッケージ変数として宣言 - Encodeのエラー処理を追加

レビュー修正

6646f83

- 終了コードを明示的に数値を代入する - flag処理をmainパッケージへ移動

レビュー修正

2ac60d3

- Converter.Executeに渡されるファイルパスの拡張子が対象外の場合ErrUnmatchExtを返すようにする - 受け取る側でErrUnmatchExtかを判断し、ErrUnmatchExtなら無視する

tenntenn reviewed Jul 20, 2020

View reviewed changes

tenntenn added the reviewed レビュー済 label Jul 20, 2020


		type JpegImage struct{}

		func (_ JpegImage) Encode(w io.Writer, m image.Image) error {

Uh oh!

Conversation

hokita commented Jul 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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

課題内容

次の仕様を満たすコマンドを作って下さい

以下を満たすように開発してください

対応したこと

動作

工夫したこと

わからなかったこと、むずかしかったこと

Uh oh!

hokita commented Jul 7, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hokita commented Jul 6, 2020 •

edited

Loading