Skip to content

Kadai1 segakazzz #1

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
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Kadai1 segakazzz #1

wants to merge 9 commits into from

Conversation

segakazzz
Copy link

課題1のレビューをお願い致します。

main packageでどこまで記述して、自作パッケージにどこまでの内容を切り出すべきか判断がよくわかっていないので、良い書き方があればコメント頂けると幸いです。

その他はkadai1/segakazzz/README.mdをご参照ください。

@@ -0,0 +1,7 @@
.idea
image/in/*
Copy link
Member

Choose a reason for hiding this comment

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

テスト用のデータはtestdata以下に置くと良いです。

dir = flag.String("d", ".", "Indicate directory to convert")
in = flag.String("i", "jpg", "Indicate input image file's extension")
out = flag.String("o", "png", "Indicate output image file's extension")
err error
Copy link
Member

Choose a reason for hiding this comment

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

ここで定義しなくても、36行目で定義されます。

flag.Parse()
c, err := newConverter(*dir, *in, *out)
if err != nil {
log.Fatal(err)
Copy link
Member

Choose a reason for hiding this comment

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

log.Fatalは内部で、os.Exit(1)を呼んでいます。
そのため、終了コードを自分で決められないので、fmt.Fprintln(os.Stderr, ....)os.Exitを自分で呼んだ方が良いです。

case "jpg", "png":
input = strings.ToLower(input)
default:
return &converter{}, fmt.Errorf("Input extension is not valid. Select one from jpg/png")
Copy link
Member

Choose a reason for hiding this comment

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

エラーの際は最後の戻り値以外はゼロ値(この場合はポインタなのでnil)を返す。

}

// Convert method converts all jpg files in dirname to png. "out" folder is generated if it doesn't exist.
func (c *converter) Convert() (e error) {
Copy link
Member

Choose a reason for hiding this comment

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

名前付き戻り値は必要な時以外は使わない

func (c *converter) getSourceFiles() ([]os.FileInfo, error) {
files, err := ioutil.ReadDir(c.dirname)
if err != nil {
return []os.FileInfo{}, err
Copy link
Member

Choose a reason for hiding this comment

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

return nil, err

return nil
}

func (c *converter) convertSingle(filename string) (e error) {
Copy link
Member

@tenntenn tenntenn Jul 6, 2020

Choose a reason for hiding this comment

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

何をreturnしたか追いづらいので、名前付き戻り値はできるだけ使わない。

func (c *converter) convertSingle(filename string) (e error) {
input := filepath.Join(c.dirname, filename)
outDir := filepath.Join(c.dirname, "out")
output := filepath.Join(outDir, strings.Replace(strings.ToLower(filename), "."+c.input, "."+c.output, -1))
Copy link
Member

Choose a reason for hiding this comment

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

Replaceまで使わなくてもスライス演算でできます。

os.Mkdir(outDir, 0755)
}

in, _ := os.Open(input)
Copy link
Member

Choose a reason for hiding this comment

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

エラーは絶対無視しない

return
}
}
defer in.Close()
Copy link
Member

Choose a reason for hiding this comment

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

outを開く前にdeferを仕掛けておく

}
}
defer in.Close()
defer out.Close()
Copy link
Member

Choose a reason for hiding this comment

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

CreateなのでCloseする際にエラー処理もする。

@tenntenn tenntenn added kadai1 課題1 reviewed レビュー済 labels Jul 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kadai1 課題1 reviewed レビュー済
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants