Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

hokita / 課題1 #5

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

Open
hokita wants to merge 4 commits into master
base: master
Choose a base branch
Loading
from kadai1-hokita
Open

hokita / 課題1 #5

hokita wants to merge 4 commits into master from kadai1-hokita

Conversation

@hokita
Copy link

@hokita hokita commented Jul 6, 2020 •
edited
Loading

課題 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 hokita changed the title (削除) create kadai1/hokita (削除ここまで) (追記) hokita / 課題1 (追記ここまで) Jul 6, 2020
Copy link
Author

hokita commented Jul 7, 2020

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

@tenntenn tenntenn added the kadai1 課題1 label Jul 9, 2020
"path/filepath"
)

func converterFactory(from string, to string) (*Converter, error) {
Copy link
Member

@tenntenn tenntenn Jul 9, 2020

Choose a reason for hiding this comment

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

from, to stringと書けます

"path/filepath"
)

func converterFactory(from string, to string) (*Converter, error) {
Copy link
Member

@tenntenn tenntenn Jul 9, 2020

Choose a reason for hiding this comment

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

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

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

@tenntenn tenntenn Jul 9, 2020

Choose a reason for hiding this comment

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

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


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

@tenntenn tenntenn Jul 9, 2020

Choose a reason for hiding this comment

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

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

hiroygo reacted with eyes emoji
Copy link
Member

@hiroygo hiroygo Jul 9, 2020

Choose a reason for hiding this comment

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

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

hokita reacted with laugh emoji
}

// output image
conv.toImage.Encode(out, img)
Copy link
Member

@tenntenn tenntenn Jul 9, 2020

Choose a reason for hiding this comment

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

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

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

@tenntenn tenntenn Jul 9, 2020

Choose a reason for hiding this comment

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

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


type JpegImage struct{}

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

@tenntenn tenntenn Jul 9, 2020

Choose a reason for hiding this comment

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

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

}

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

@tenntenn tenntenn Jul 9, 2020

Choose a reason for hiding this comment

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

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

Copy link
Member

@tenntenn tenntenn Jul 9, 2020

Choose a reason for hiding this comment

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

以下同様です。

)

const (
ExitCodeOk int = iota
Copy link
Member

@tenntenn tenntenn Jul 9, 2020

Choose a reason for hiding this comment

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

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

hokita added 3 commits July 11, 2020 15:50 
- 引数を同じ型でまとめる
- Close()のエラー処理を追加
- factoryメソッド名をnewを付与
- 拡張子をmapのパッケージ変数として宣言
- Encodeのエラー処理を追加
- 終了コードを明示的に数値を代入する
- flag処理をmainパッケージへ移動
- Converter.Executeに渡されるファイルパスの拡張子が対象外の場合ErrUnmatchExtを返すようにする
 - 受け取る側でErrUnmatchExtかを判断し、ErrUnmatchExtなら無視する
return &Converter{fromImage, toImage}, nil
}

var ErrUnmatchExt = errors.New("ext does not match")
Copy link
Member

Choose a reason for hiding this comment

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

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


// file open
file, err := os.Open(path)
defer file.Close()
Copy link
Member

Choose a reason for hiding this comment

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

defer Closeはエラー処理のあと


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

Choose a reason for hiding this comment

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

defer Closeはエラー処理のあと


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

Choose a reason for hiding this comment

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

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

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

Choose a reason for hiding this comment

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

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

}

func selectImage(ext string) (ImageType, error) {
pngImage := PngImage{}
Copy link
Member

Choose a reason for hiding this comment

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

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

}

err = filepath.Walk(dir,
func(path string, info os.FileInfo, err error) error {
Copy link
Member

Choose a reason for hiding this comment

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

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


const QUALITY = 100

type JpegImage struct{}
Copy link
Member

Choose a reason for hiding this comment

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

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

"io"
)

const QUALITY = 100
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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

@tenntenn tenntenn added the reviewed レビュー済 label Jul 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tenntenn tenntenn tenntenn left review comments

@hiroygo hiroygo hiroygo left review comments

Assignees

No one assigned

Labels

kadai1 課題1 reviewed レビュー済

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /