# Wataru / boru 課題1 #2

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

Jump to bottom

Open

wataboru wants to merge 3 commits into master

from kadai1-wataboru

Open

# Wataru / boru 課題1 #2

wataboru wants to merge 3 commits into master from kadai1-wataboru

Conversation

@wataboru

Copy link

@wataboru wataboru commented Jul 5, 2020 •

edited

Loading

課題1のPRになります。レビューをお願いいたします。

以下README.mdより転記

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

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

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

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

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

実行手順

$ go build -o imageconverter
$ ./imgconv -h 
Usage of ./imgconv:
 -a string
 	Input extension after conversion. (short) (default "png")
 -after --after=jpg
 	Input extension after conversion.
 	 ex) --after=jpg (default "png")
 -b string
 	Input extension before conversion. (short) (default "jpg")
 -before --before=png
 	Input extension before conversion.
 	 ex) --before=png (default "jpg")
 -d string
 	Input target Directory. (short)
 -dir --dir=./convert_image
 	Input target Directory.
 	 ex) --dir=./convert_image
$ ./imgconv -d ./image 
 or
$ ./imgconv -d ./image -b png -a gif
 or
$ ./imgconv -d ./image -b jpeg -a tiff

前提として、@tenntennさんの公開されているhandsonプロジェクトと似ている内容でして、以前やったことがありました。
- そちらに無い部分として、tiff変換やbmp変換の追加などを行いました。
mainパッケージと自作パッケージの切り分けは上記のプロジェクトを参考にしました。
flagの扱い方は、改訂2版みんなのGo言語を参考にしました
GoModulesを利用したく、golang.org/x/imageを導入しました
様々なソースをみていると、変数名やコマンド名が自分は冗長かな?と感じています。Go話者の方は短く記載するものなのでしょうか。

wataboru added 2 commits

July 5, 2020 22:16

@wataboru


 Initial commit.

9a08ed3

@wataboru


 Add README.md

9e68451

@wataboru wataboru changed the title ~~(削除) # wataboru 課題1 (削除ここまで)~~ (追記) # Wataru / boru 課題1 (追記ここまで)

Jul 5, 2020

tenntenn

tenntenn reviewed

Jul 5, 2020

View reviewed changes

Copy link

Member

@tenntenn tenntenn left a comment

Choose a reason for hiding this comment

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

レビューしました!

kadai1/wataboru/.gitignore Outdated

@@ -0,0 +1,3 @@

.idea

.DS_Store

go.sum No newline at end of file

Copy link

Member

@tenntenn tenntenn Jul 5, 2020

Choose a reason for hiding this comment

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

ignoreする必要はないですね

kadai1/wataboru/commands/imageconverter/main.go Outdated

package main

import (

"dojo8/kadai1/wataboru/imageconverter"

Copy link

Member

@tenntenn tenntenn Jul 5, 2020

Choose a reason for hiding this comment

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

github.comからのパスの方が好ましいです。

kadai1/wataboru/commands/imageconverter/main.go Outdated

const (

// ExitCodeSuccess is the exit code on success

ExitCodeSuccess int = iota

Copy link

Member

@tenntenn tenntenn Jul 5, 2020

Choose a reason for hiding this comment

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

終了コードは外部に伝えるものなので iota は使わない方が安全です。
iota を使う値は区別さえできればOKで値として意味がないものに限ります。
終了コードは0が成功、それ以外が失敗という値に意味を持っているので使わないほうがが良いです。
DBに入れる値なんかも同様です。

kadai1/wataboru/README.md Outdated

ex) --dir=./convert_image

$ ./imgconv -d ./image

$ ./imgconv -d ./image -b png -a gif

Copy link

Member

@tenntenn tenntenn Jul 5, 2020

Choose a reason for hiding this comment

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

テスト用の画像は testdata というディレクトリを作ってその下に入れましょう。
testdata という名前のディレクトリはGoにパッケージとして認識されません。
同様に_で始まるディレクトリ(よく_exampleとして使用される)や.で始まるディレクトリも無視されます。

kadai1/wataboru/README.md

- 前提として、@tenntennさんの公開されている[handsonプロジェクト](https://github.com/tenntenn/gohandson/tree/master/imgconv/ja)と似ている内容でして、以前やったことがありました。

- そちらに無い部分として、tiff変換やbmp変換の追加などを行いました。

- GoModulesを利用したく、`golang.org/x/image`を導入しました

Copy link

Member

@tenntenn tenntenn Jul 5, 2020

Choose a reason for hiding this comment

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

👍

kadai1/wataboru/imageconverter/ImageConverter.go Outdated

"strings"

"golang.org/x/image/tiff"

Copy link

Member

@tenntenn tenntenn Jul 5, 2020

Choose a reason for hiding this comment

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

詰める

kadai1/wataboru/imageconverter/ImageConverter.go Outdated

if err != nil {

return fmt.Errorf("image file could not be created. target: %s", dest)

}

defer destFile.Close()

Copy link

Member

@tenntenn tenntenn Jul 5, 2020

Choose a reason for hiding this comment

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

Create は書き込みの処理なので、Close する際にもエラー処理をする。

kadai1/wataboru/imageconverter/ImageConverter.go Outdated

func Convert(args Args) error {

return filepath.Walk(args.Directory, func(path string, info os.FileInfo, err error) error {

if err != nil {

fmt.Printf("prevent panic by handling failure accessing a path %q: %v\n", path, err)

Copy link

Member

@tenntenn tenntenn Jul 5, 2020

Choose a reason for hiding this comment

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

エラーをreturnするならログを出さない。
ログを出すならエラーを返さない。
エラーログの出力もエラーハンドリングの1つでエラーハンドリングは1度だけにする。
出すとしても標準エラー出力の方が好ましそう。

@wataboru


 - Change example directory name "image" to "testdata"

350ed73

- Modify import statement in main.go.
- Add error handling to defer of imageconverter.convertImage.

wataboru

wataboru commented

Jul 8, 2020

View reviewed changes

kadai1/wataboru/imageconverter/ImageConverter.go

defer func(returnErr error) {

if returnErr == nil {

err = destFile.Close()

Copy link

Author

@wataboru wataboru Jul 8, 2020

Choose a reason for hiding this comment

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

@tenntenn
こちら、分かる限り修正してみました。
一点質問がありまして、defer内で呼び出し元にエラーを返す処理を実現するため、名前付き戻り値を使ってしまっています。

他の方のレビューの中で、「何をreturnしたか追いづらいので、名前付き戻り値はできるだけ使わない。」とあったので、名前付き戻り値を使わずにどのように呼び出し元にエラーを返したら良いか教えていただきたいです。

Copy link

Member

@tenntenn tenntenn Jul 8, 2020

Choose a reason for hiding this comment

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

このパターンのみ使わないとどうにもならないので使っても大丈夫です!

Copy link

Author

@wataboru wataboru Jul 8, 2020

Choose a reason for hiding this comment

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

@tenntenn
なるほど!そうなんですね。
一応その他の場所は明示的にreturnに渡す様にしました。

@tenntenn tenntenn added kadai1 reviewed labels

Jul 9, 2020

Labels

kadai1 reviewed

2 participants

@wataboru @tenntenn

# Wataru / boru 課題1 #2

Are you sure you want to change the base?

# Wataru / boru 課題1 #2

Uh oh!

Conversation

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

Uh oh!

以下README.mdより転記

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

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

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

実行手順

コメント

Uh oh!

@tenntenn tenntenn left a comment

Choose a reason for hiding this comment

Uh oh!

@tenntenn tenntenn Jul 5, 2020

Choose a reason for hiding this comment

Uh oh!

@tenntenn tenntenn Jul 5, 2020

Choose a reason for hiding this comment

Uh oh!

@tenntenn tenntenn Jul 5, 2020

Choose a reason for hiding this comment

Uh oh!

@tenntenn tenntenn Jul 5, 2020

Choose a reason for hiding this comment

Uh oh!

@tenntenn tenntenn Jul 5, 2020

Choose a reason for hiding this comment

Uh oh!

@tenntenn tenntenn Jul 5, 2020

Choose a reason for hiding this comment

Uh oh!

@tenntenn tenntenn Jul 5, 2020

Choose a reason for hiding this comment

Uh oh!

@tenntenn tenntenn Jul 5, 2020

Choose a reason for hiding this comment

Uh oh!

@wataboru wataboru Jul 8, 2020

Choose a reason for hiding this comment

Uh oh!

@tenntenn tenntenn Jul 8, 2020

Choose a reason for hiding this comment

Uh oh!

@wataboru wataboru Jul 8, 2020

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

@wataboru wataboru commented Jul 5, 2020 •

edited

Loading