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

tsuchinaga / 課題1 #3

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions kadai1/tsuchinaga/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
### for IntelliJ
.idea
*.iml
41 changes: 41 additions & 0 deletions kadai1/tsuchinaga/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# 課題1 tsuchinaga

## 要件

### 次の仕様を満たすコマンドを作って下さい
* ディレクトリを指定する
* 指定したディレクトリ以下のJPGファイルをPNGに変換(デフォルト)
* ディレクトリ以下は再帰的に処理する
* 変換前と変換後の画像形式を指定できる(オプション)

### 以下を満たすように開発してください
* mainパッケージと分離する
* 自作パッケージと標準パッケージと準標準パッケージのみ使う
* 準標準パッケージ:golang.org/x以下のパッケージ
* ユーザ定義型を作ってみる
* GoDocを生成してみる
* Go Modulesを使ってみる

## TODO
* [x] コマンドラインで実行できる
* [x] パラメータでディレクトリを指定できる
* [x] ディレクトリの指定がない場合はディレクトリを指定するようにメッセージを出して終了
* [x] ディレクトリではなくファイルを指定されたらディレクトリを指定するようメッセージを出して終了
* [x] 指定されたディレクトリ、ファイルが存在しない場合は存在しない旨をメッセージを出して終了
* [x] 変換元の画像形式が指定できる
* [x] 指定しなかった場合はJPG
* [x] 指定可能な画像形式はJPG、PNG
* [x] 指定可能でない画像形式が指定されたら指定できる画像形式をメッセージで出して終了
* [x] 変換後の画像形式が指定できる
* [x] 指定しなかった場合はPNG
* [x] 指定可能な画像形式はJPG、PNG
* [x] 指定可能でない画像形式が指定されたら指定できる画像形式をメッセージで出して終了
* [x] 変換元と同じ画像形式を指定されたら違い画像形式を選ぶようメッセージを出して終了
* [x] 指定されたディレクトリの配下にある変換元で指定された画像形式の画像を、変換後で指定された画像形式の画像に変換する
* [x] 指定されたディレクトリの配下にサブディレクトリがあればサブディレクトリ内の画像に対しても変換する
* [x] サブディレクトリ内のディレクトリに対しても同様に処理する = 指定されたディレクトリ配下に対して再帰的に実行する
* [x] 指定されたディレクトリ配下に画像がない、もしくは指定された画像形式のファイルがなければ何もしない
* [x] 画像の変換ができる
* [x] 変換元画像が開けなかったらその旨を表示して次のファイルに進む
* [x] 変換後の画像が作れなかったらその旨を表示して次のファイルに進む
* [x] 変換出来たら変換前のパスと変換後のパスを表示して次のがいるに進む
125 changes: 125 additions & 0 deletions kadai1/tsuchinaga/conv/converter.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
package conv

import (
"fmt"
"image"
"image/jpeg"
"image/png"
"io/ioutil"
"log"
"os"
"path/filepath"
)

var validFileTypes = []string{"jpeg", "png"}
Copy link
Member

Choose a reason for hiding this comment

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

mapを使った方がシンプルになりますね。

var validFileTypes = map[string]bool{"jpeg": true, "png": true}

func IsValidFileType(fileType string) bool {
        return validFileTypes[fileType]
}


// IsValidFileType - 指定されたファイルタイプが利用可能かを返す
func IsValidFileType(fileType string) bool {
for _, t := range validFileTypes {
if t == fileType {
return true
}
}
return false
}

// IsDir - pathがディレクトリかどうか
func IsDir(path string) bool {
_, err := ioutil.ReadDir(path)
return err == nil
Copy link
Member

Choose a reason for hiding this comment

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

これではディレクトリかどうかは判断できませんね。
読み込めないエラーには単にストレージが壊れていたりする場合も含まれますので。
os.Statとか使ってos.FileInfoを取得してIsDirメソッドで確認した方がいいです。

}

// ExecConvert - 変換の実行
func ExecConvert(dir, src, dest string) {
Copy link
Member

Choose a reason for hiding this comment

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

conv.ExecConvertとなり、意味が二重になります。
Goは他のパッケージの関数などを呼ぶ際は必ず(ほぼ)、パッケージ名も合わせて記載するため、それも込みで名前をつける必要があります。
imgconv.Doとかであれえば、意味の重複もなく、十分意味が伝わるでしょう。

c := converter{
dirList: []string{dir},
srcFileType: src,
destFileType: dest,
}
c.exec()
}

// converter - 変換機能の実装
type converter struct {
dirList []string
srcFileType string
destFileType string
}

// exec - ディレクトリをたどりながら変換を実行
func (c *converter) exec() {
for len(c.dirList) > 0 {
dirPath := c.dirList[0]
c.dirList = c.dirList[1:]

files, err := ioutil.ReadDir(dirPath)
if err != nil {
log.Printf("ディレクトリ: %sが読み込めなかったためスキップします\n", dirPath)
Copy link
Member

Choose a reason for hiding this comment

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

ライブラリとして使うパッケージが標準出力や標準エラー出力にメッセージを出すのはやりすぎかなと思います。
読み込めない場合はちゃんとエラーとして呼び出し元に返し、呼び出し元でどうするか判断してもらったほうが良いです。

continue
}

for _, file := range files {
path := filepath.Join(dirPath, file.Name())
if file.IsDir() {
c.dirList = append(c.dirList, path)
} else {
c.convert(path)
}
}
}
}

// convert - 変換処理
func (c converter) convert(path string) {
f, err := os.Open(path)
if err != nil { // 開けない
log.Printf("ファイル: %sが開けなかったためスキップします\n", path)
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 f.Close()

if getFileType(path) == c.srcFileType {
img, _, err := image.Decode(f)
if err != nil {
log.Printf("ファイル: %sが読み込めなかったためスキップします\n", path)
return
}

newFilePath := fmt.Sprintf("%s.%s", path, c.destFileType)
o, err := os.Create(newFilePath)
if err != nil {
log.Printf("変換後ファイル: %sが作成できなかったためスキップします\n", newFilePath)
return
}
defer o.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のエラー処理をする


err = nil
Copy link
Member

Choose a reason for hiding this comment

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

case句でそれぞれreturnした方が分かりやすいかと思います

switch c.destFileType {
case "jpeg":
err = jpeg.Encode(o, img, nil)

case "png":
err = png.Encode(o, img)
}
if err != nil {
log.Printf("ファイル: %sの変換に失敗しました\n", path)
} else {
log.Printf("%s => %s\n", path, newFilePath)
Copy link
Member

Choose a reason for hiding this comment

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

logパッケージはデフォルトでは標準エラー出力に出力を行います。
そのため、エラーではない進捗などは標準出力に出力すべきです。
なお、ライブラリなので、進捗の出力はmainパッケージに任せたほうがよいでしょう。
ライブラリ側は進捗が出力できるように工夫するほうが良いです。

}
}
}

// getFileType - 画像ファイルの型を得る
func getFileType(path string) string {
f, err := os.Open(path)
if err != nil { // 開けない
Copy link
Member

Choose a reason for hiding this comment

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

エラーは適切にハンドルしてください。
エラーはすでに値なのでエラーを空文字で表現せずに、そのまま多値でreturnしてください。

return ""
}
defer f.Close()

_, format, err := image.DecodeConfig(f)
if err != nil { // 画像じゃない
return ""
}
return format
}
3 changes: 3 additions & 0 deletions kadai1/tsuchinaga/go.mod
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module github.com/gopherdojo/dojo8/kadai1/tsuchinaga

go 1.14
35 changes: 35 additions & 0 deletions kadai1/tsuchinaga/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package main

import (
"flag"
"github.com/gopherdojo/dojo8/kadai1/tsuchinaga/conv"
Copy link
Member

Choose a reason for hiding this comment

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

goimportsを使いましょう。

"log"
)

func main() {
var dir, src, dest string
flag.StringVar(&dir, "dir", "", "変換する画像のあるディレクトリ")
flag.StringVar(&src, "src", "jpeg", "optional 変換元の画像形式 jpeg|png")
flag.StringVar(&dest, "dest", "png", "optional 変換後の画像形式 jpeg|png")
flag.Parse()

// validation
if dir == "" {
log.Fatalln("dirの指定は必須です")
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.Exitを使うようにしましょう。

}
if !conv.IsDir(dir) {
log.Fatalf("%sは存在しないかディレクトリではありません\n", dir)
}
if !conv.IsValidFileType(src) {
log.Fatalf("%sは許可されていない画像形式です\n", src)
}
if !conv.IsValidFileType(dest) {
log.Fatalf("%sは許可されていない画像形式です", dest)
}
if src == dest {
log.Fatalln("srcとdestで違う画像形式を選択してください")
}

// 変換実行
conv.ExecConvert(dir, src, dest)
Copy link
Member

Choose a reason for hiding this comment

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

変換を行っているのにエラー処理がmain関数にないのは不自然です。
エラー処理は必ず呼び出し元が行うようにしましょう。

}
Binary file added kadai1/tsuchinaga/testdata/Example.jpg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added kadai1/tsuchinaga/testdata/Example.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added kadai1/tsuchinaga/testdata/subdir1/Example.jpg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added kadai1/tsuchinaga/testdata/subdir1/Example.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.