Skip to content

【課題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

Open
wants to merge 31 commits into
base: master
Choose a base branch
from

Conversation

kz23szk
Copy link

@kz23szk kz23szk commented Sep 4, 2019

やったこと

  • 画像変換コマンドを作成
  • main以外に画像変換用パッケージ(image)を作成
  • godocのページをpdf化

進捗

  • png, jpg(jpeg), gifの相互変換ができるところまで

一日経って見返して下記改善したい。

  • ".png" <=> "png"の変換もうちょっとうまく行かないか 

特に見てもらいたい点

  • 変数や関数名などの長さ
    • ライブラリを見ているとかなり短めでなるべく省略しているように見えるが、全部うまく省略できるものばかりでないのでそういった場合の方針とか考え方があれば教えてほしいです。(共通認識の取れているもの(img, cnfとか)は短く、それ以外はあまり気にしなくてもよい、とか)
  • 対応フォーマットのチェック部分(https://github.com/gopherdojo/dojo7/blob/fec0bc1351db4f5a9ba62363c64020fb485df81d/kadai1/su-san/image/image.go#L13-L18)
    • 配列にしてcontainsみたいな関数を作ったほうがよいか?(講義中のカンマOK記法がピッタリだと思ったのですがvalueは完全に無駄なのでどうか)

@kz23szk kz23szk added the kadai1 label Sep 4, 2019
@kz23szk kz23szk self-assigned this Sep 4, 2019
"path/filepath"
)

var supportedFormats = map[string]int{"jpg": 1, "jpeg": 1, "png": 1, "gif": 1}
Copy link
Member

Choose a reason for hiding this comment

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

値はintじゃなくて、boolの方が便利です(ゼロ値がfalseなのでキーが無ければfalseになります)

Copy link
Member

Choose a reason for hiding this comment

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

okで判断する場合はstruct{}の方が軽い

Copy link
Author

Choose a reason for hiding this comment

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

int型に意味はないためbool型に修正しました。
intだと1以外の数値があると推測されるため。

}

// SupportedFormats は指定フォーマットが対応しているか確認するメソッドです
func (c ConvExts) SupportedFormats() bool {
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
Author

Choose a reason for hiding this comment

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

構造体の中身を更新しない場合はポインタにしなくてもよいと思ってましたが、
コピーが発生するため基本的にポインタの方が良いのですね。
修正しました!

}

// FmtConv は指定されたフォーマットからフォーマットへ変換する関数です
func FmtConv(path string, exts ConvExts) (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.

特別な用途が無い限りは、名前付き戻り値はあまり使わない

  • 名前付き戻り値でしかできない場合
  • 戻り値が(型で)分かりづらい場合にドキュメント的に

Copy link
Author

Choose a reason for hiding this comment

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

直しました!

// FmtConv は指定されたフォーマットからフォーマットへ変換する関数です
func FmtConv(path string, exts ConvExts) (err error) {
pathWithoutExt := path[:len(path)-len(filepath.Ext(path))+1]
ext := filepath.Ext(path)[1:]
Copy link
Member

Choose a reason for hiding this comment

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

拡張子がない場合は?

}

outputFile, err := os.Create(pathWithoutExt + exts.outExt)
defer outputFile.Close()
Copy link
Member

@tenntenn tenntenn Sep 9, 2019

Choose a reason for hiding this comment

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

Closeより、errのチェックを先にする

書き込みの場合はCloseのエラー処理をする。

encodeErr = gif.Encode(outputFile, img, nil)
}

if encodeErr == 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したほうがわかりやすい。

if encodeErr != nil {
    return encodeErr
}

}

if encodeErr == nil {
err = os.Remove(path)
Copy link
Member

Choose a reason for hiding this comment

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

結局errに代入しても意味がない。
こうするなら

err = os.Remove(path)
return

としなければならない

import (
"flag"
"fmt"
//"io/ioutil"
Copy link
Member

Choose a reason for hiding this comment

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

コメントアウトせずにgoimportsを使う。
PR投げる際には意図のあるコメント以外は書かない

Copy link
Author

Choose a reason for hiding this comment

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

失礼しました。修正しました!


convExts := image.NewConvExts(*inputExt, *outputExt)
if convExts.SupportedFormats() == false {
fmt.Println("unsupported format! please specify these format [png jpg jpeg gif]")
Copy link
Member

Choose a reason for hiding this comment

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

エラーならos.Stderrに出す

Copy link
Author

Choose a reason for hiding this comment

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

修正しました!

}

convExts := image.NewConvExts(*inputExt, *outputExt)
if convExts.SupportedFormats() == false {
Copy link
Member

Choose a reason for hiding this comment

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

if !convExts.SupportedFormats()と書く

Copy link
Author

Choose a reason for hiding this comment

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

どちらが一般的な書き方かわかっていませんでした。
修正しました。

return
}

targetFiles := []string{}
Copy link
Member

Choose a reason for hiding this comment

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

ゼロ値で大丈夫。appendnilを許容する

Copy link
Author

Choose a reason for hiding this comment

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

修正しました。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants