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

Wataru / boru 課題2 #11

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

Wataru / boru 課題2 #11

wants to merge 5 commits into from

Conversation

wataboru
Copy link

@wataboru wataboru commented Jul 13, 2020

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

以下README.mdより転記

課題 2

【TRY】io.Readerとio.Writer

io.Readerとio.Writerについて調べてみよう

  • 標準パッケージでどのように使われているか
  • io.Readerとio.Writerがあることでどういう利点があるのか具体例を挙げて考えてみる

長文のため畳みます > - 標準パッケージでどのように使われているか
  • 実装している標準パッケージ

    • bufio
    • bytes
    • zip
    • strings
  • どの様に使われているか

    • strings
func (w *appendSliceWriter) Write(p []byte) (int, error) {
	*w = append(*w, p...)
	return len(p), nil
}
  • bufio
func (b *Writer) Write(p []byte) (nn int, err error) {
	for len(p) > b.Available() && b.err == nil {
		var n int
		if b.Buffered() == 0 {
			// Large write, empty buffer.
			// Write directly from p to avoid copy.
			n, b.err = b.wr.Write(p)
		} else {
			n = copy(b.buf[b.n:], p)
			b.n += n
			b.Flush()
		}
		nn += n
		p = p[n:]
	}
	if b.err != nil {
		return nn, b.err
	}
	n := copy(b.buf[b.n:], p)
	b.n += n
	nn += n
	return nn, nil
}
  • zip
func (w *fileWriter) Write(p []byte) (int, error) {
	if w.closed {
		return 0, errors.New("zip: write to closed file")
	}
	w.crc32.Write(p)
	return w.rawCount.Write(p)
}
  • io.Readerとio.Writerがあることでどういう利点があるのか具体例を挙げて考えてみる
  1. 各パッケージで扱っている書き込み先は全て違うが、同じインターフェースにて抽象化されていること中の実装は隠蔽され、利用者としてはそれぞれ全て等しく WriteRead で利用することができる。
  2. 上記の通り隠蔽されることで、書き込み先や読み込み先という仕様が変更になったとしても、利用するパッケージを変更するだけで実現が可能になる。(変更容易性が上がる)

【TRY】テストを書いてみよう

1回目の課題のテストを作ってみて下さい

  • テストのしやすさを考えてリファクタリングしてみる
  • テストのカバレッジを取ってみる
  • テーブル駆動テストを行う
  • テストヘルパーを作ってみる

動作手順

$ go test ./imageconverter
$ go test ./commands/imageconverter

カバレッジ

  • imageconverter

    • ./imageconverter_coveage.html
  • main

    • ./imageconverter_coveage.html

感想等

io.Readerとio.Writerについて調べてみよう

  • 一般的な抽象化のメリットを書いてしまっていて、Goならではのポイントに気づくことができませんでした...

【TRY】テストを書いてみよう

  • 画像のエンコード処理を、ストラテジーパターンを利用してリファクタリングしました。
  • 可能な限り、テスト対象の関数内で利用しているパッケージをMockして、依存を減らす様にしました。(Mockのパッケージを使わないで実現するのに相当苦労しました。)
  • 標準パッケージや自身の実装した関数のmockにパッケージ変数とinitを利用していますが、これで正しいか悩んでいます。
  • os.FileInfoはInterfaceのため、test側でモックを実装しています。が、不要なメソッドも実装する必要があり苦労しました。埋め込みを用いた上で該当するメソッドだけ修正することは可能なのでしょうか。
  • mainのテストにて、flag.Parse() をinitの中で実行していたため、testでflag引数を渡すことができず苦戦しました。こちら、run()側にflag.Parse() を渡すのか、 testing.Init() を呼び出すか、どちらが良いか悩んでいます。あまりテストコードがメインの実装に漏れるのが嫌なので、他の方法があれば知りたいです。
  • テスタブルなコードを目指すあまり、実装として読みにくくなっていないか気になっています。

AfterExtension string
}

type Encoder interface {
Copy link
Author

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 left a comment

Choose a reason for hiding this comment

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

個人的な不明点をGithubコメントとして記載しました。

destFileClose func(file *os.File) error
imageDecode func(r io.Reader) (image.Image, string, error)
newImgEncoder func(ext string) (Encoder, error)
)
Copy link
Author

Choose a reason for hiding this comment

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

標準パッケージや自身の実装した関数のmockにパッケージ変数とinitを利用していますが、これで正しいか悩んでいます。

Copy link
Author

Choose a reason for hiding this comment

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

ただ可能な限り、テスト対象の関数内で利用しているパッケージをMockして、依存を減らす様にしました。(Mockのパッケージを使わないで実現するのに相当苦労しました。)

return fmt.Errorf("error")
}

type FileInfoMock struct{}
Copy link
Author

Choose a reason for hiding this comment

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

os.FileInfoはInterfaceのため、test側でモックを実装しています。が、不要なメソッドも実装する必要があり苦労しました。埋め込みを用いた上で該当するメソッドだけ修正することは可能なのでしょうか。

)

func init() {
testing.Init()
Copy link
Author

Choose a reason for hiding this comment

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

mainのテストにて、flag.Parse() をinitの中で実行していたため、testでflag引数を渡すことができず苦戦しました。こちら、run()側にflag.Parse() を渡すのか、 testing.Init() を呼び出すか、どちらが良いか悩んでいます。あまりテストコードが実装に漏れるのが嫌なのですが。。。他の方法があれば知りたいです。

@wataboru wataboru requested a review from tenntenn July 13, 2020 22:25
@wataboru wataboru added the kadai2 課題2 label Jul 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kadai2 課題2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant