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

Kadai3 tomoyuki.kobayashi #36

Open
wants to merge 41 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
8a7dcf1
add kadai1 files :lol:
Nov 8, 2018
ab31386
add todos
Nov 8, 2018
ef36126
add kadai2 files
Nov 17, 2018
c9587ee
Delete .gitignore
bakisunsan Nov 17, 2018
e69b664
Delete Makefile
bakisunsan Nov 17, 2018
9e5a807
Delete README.md
bakisunsan Nov 17, 2018
f9f7c4e
Delete main.go
bakisunsan Nov 17, 2018
4bf0dd9
Delete file_finder.go
bakisunsan Nov 17, 2018
71569f9
Delete formats.go
bakisunsan Nov 17, 2018
4cec624
Delete formats_test.go
bakisunsan Nov 17, 2018
4458643
Delete image_converter.go
bakisunsan Nov 17, 2018
d7df320
Update main.go
bakisunsan Nov 18, 2018
e5de4ee
add load question logics
Nov 21, 2018
06ad2ec
とりあえず一通り動くバージョン
Nov 22, 2018
2abbf61
コメント修正
Nov 22, 2018
3e212f5
Delete Makefile
bakisunsan Nov 22, 2018
8bfcc95
Delete README.md
bakisunsan Nov 22, 2018
1b37ffa
Delete file_finder.go
bakisunsan Nov 22, 2018
788ceb5
Delete file_finder_test.go
bakisunsan Nov 22, 2018
bf5cb5e
Delete formats.go
bakisunsan Nov 22, 2018
26db9b9
Delete formats_test.go
bakisunsan Nov 22, 2018
74dd45a
Delete image_converter.go
bakisunsan Nov 22, 2018
c074309
Delete image_converter_test.go
bakisunsan Nov 22, 2018
d8e8288
Delete main.go
bakisunsan Nov 22, 2018
06c5785
Delete main_test.go
bakisunsan Nov 22, 2018
00001a7
Update Makefile
bakisunsan Nov 22, 2018
4f8b436
refactor game
Nov 22, 2018
89975a8
リファクタリングとテスト追加
Nov 22, 2018
feb6bd9
Merge branch 'kadai3-tomoyuki.kobayashi' of https://github.com/bakisu…
Nov 22, 2018
4b10c87
rename
Nov 22, 2018
3dbe9fb
add test
Nov 22, 2018
5decc01
rangeの動作確認ができてる版
Nov 23, 2018
1a4401c
rangeダウンロードできる版
Nov 24, 2018
d4abbe7
work version
Nov 24, 2018
ce761c1
work version
Nov 24, 2018
7a6cd63
Delete file4.txt
bakisunsan Nov 24, 2018
28b3f22
Delete invalid.jpg
bakisunsan Nov 24, 2018
acefdc7
ファイルクローズが漏れていたので追加
Nov 24, 2018
847d7c4
Merge branch 'kadai3-tomoyuki.kobayashi' of https://github.com/bakisu…
Nov 24, 2018
e06f9a4
add defer
Nov 25, 2018
67258fe
fix for review comments
Nov 27, 2018
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
Binary file added kadai2/src/tomoyukikobayashi/testdata/file1.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 kadai2/src/tomoyukikobayashi/testdata/file2.jpeg
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 kadai2/src/tomoyukikobayashi/testdata/file3.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Empty file.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
25 changes: 25 additions & 0 deletions kadai3-1/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
.PHONY: deps
deps:
go get github.com/golang/lint/golint

.PHONY: build
build:
go build -o type tomoyukikobayashi

.PHONY: test
test:
go test -v -cover ./...

.PHONY: cover
cover:
go test -coverprofile=mainprof tomoyukikobayashi
go tool cover -html=mainprof

.PHONY: lint
lint: deps
go vet ./...
golint ./...

.PHONY: fmt
fmt: deps
go fmt *.go
19 changes: 19 additions & 0 deletions kadai3-1/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
GoImageConverter
=====

# Overview

TODO

# SetUp

下記のようにコマンドを叩くと、実行形式のTODOファイルが生成されます
```
cd kadai2
make build
```

# Usage


# 課題に対するアプローチ
70 changes: 70 additions & 0 deletions kadai3-1/src/tomoyukikobayashi/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
/*
Pacakge main is the entry point of this project.
This mainly provides interaction logics and parameters
used in CLI intrerfaces.
*/
package main

import (
"fmt"
"io"
"os"

"tomoyukikobayashi/typing"
)

// CLIのExitコード
const (
ExitSuccess = 0
ExitError = 1
)

// Exitしてもテスト落ちない操作するようにエイリアスにしている
var exit = os.Exit

// CLI テストしやすいようにCLIの出力先を差し替えられるようにしている
type CLI struct {
inStream io.Reader
outStream io.Writer
errStream io.Writer
}

// CLIツールのエントリーポイント
func main() {
cli := &CLI{inStream: os.Stdin, outStream: os.Stdout, errStream: os.Stderr}
exit(cli.Run(os.Args))
}

// Run テスト用に実行ロジックを切り出した内容
func (c *CLI) Run(args []string) int {

game, err := typing.NewGame()
if err != nil {
fmt.Fprintf(c.outStream, "failed to initizalize game %v", err)
return ExitError
}

// TOOD 長いことテスト固めたくないので外から与えるようにする
fmt.Fprintf(c.outStream, "start game 10 sec\n")
qCh, aCh, rCh := game.Run(10, c.inStream)

for {
q, progress := <-qCh
fmt.Fprintf(c.outStream, "%v\n", q)
if !progress {
break
}

fmt.Fprintf(c.outStream, ">")
a, progress := <-aCh
fmt.Fprintf(c.outStream, "%v\n", a)
if !progress {
break
}
}

r := <-rCh
fmt.Fprintf(c.outStream, "clear %v miss %v\n", r[0], r[1])

return ExitSuccess
}
107 changes: 107 additions & 0 deletions kadai3-1/src/tomoyukikobayashi/typing/game.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
package typing

import (
"bufio"
"context"
"io"
"time"
)

// Game タイピングゲームを制御するロジックを返す
type Game interface {
Run(int, io.Reader) (question <-chan string,
answer <-chan string, result <-chan [2]int)
}

type constGame struct {
Questioner
// NOTE コンテキストに状態格納しようとしたがコピーして作り直すから、途中で書き換えてもルーチンをまたいでシェアされない模様
// 結局共有変数に書いていることになるので、脆弱感がすごい
clear int
miss int
currentWord string
}

// NewGame Gameのコンストラクタです
func NewGame() (Game, error) {
q, err := NewQuestioner()
if err != nil {
return nil, err
}
return &constGame{Questioner: q}, nil
}

// Run ゲームを開始する
func (c *constGame) Run(timeout int, input io.Reader) (<-chan string,
<-chan string, <-chan [2]int) {
sc := bufio.NewScanner(input)

qCh := make(chan string)
aCh := make(chan string)
routines := 2
rCh := make(chan [2]int, routines)

bc := context.Background()
tc := time.Duration(timeout) * time.Second
ctx, _ := context.WithTimeout(bc, tc)
// NOTE defer cancel() defer cancelするとDone条件閉じてる

// TODO 読みづらいので関数に切り出す
go func() {
for {
word := c.GetNewWord(c.nextLevel())
qCh <- word
Copy link

Choose a reason for hiding this comment

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

ここでブロックしてしまうと、問題がchanから読み込まれるまでcontextによるキャンセルが効かなくなるので、selectに含めるとより扱いやすくなると思います

Copy link
Author

Choose a reason for hiding this comment

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

そうですね。selectにこれを追加して、defaultをなくすのが良さそうです
ありがとうございます。

c.currentWord = word
Copy link

Choose a reason for hiding this comment

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

この c.currentWord の扱いについて、このgoroutineも参照して、answerのgoroutineでも isCorrect のメソッドから参照しているのでおそらくraceしてしまいそうです(おそらくまず発生しないと思いますが)
go test -race でテストを実行してみてください

おそらく発生する流れ

  • aCh <- ansが実行される
  • mainのforのblockが次に進むので、qChから読み込まれ、c.currentWordが更新される
  • isCorrectで次の問題の答えと比較されて、間違いと判定されてしまう

Copy link
Author

Choose a reason for hiding this comment

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

おそらくまず発生しないと思いますが

テストで固定で複数行のstringをinputさせて動作させてみたら結果に揺れがあったので、なんでだろうと思っていましたが
多分こいつですね。。。普通に起きていました

select {
case <-ctx.Done():
// TODO once.DO が使えるかも
rCh <- [2]int{c.clear, c.miss}
close(qCh)
return
default:
//do nothing
}
}
}()

// 読み込みしてaChに送る
go func() {
for {
if sc.Scan() {
ans := sc.Text()
aCh <- ans
if c.isCorrect(ans) {
c.clear = c.clear + 1
} else {
c.miss = c.miss + 1
}
}

select {
case <-ctx.Done():
rCh <- [2]int{c.clear, c.miss}
close(aCh)
return
default:
//do nothing
}
}
}()

return qCh, aCh, rCh
}

// HACK 成功した回数に応じて、使う語彙のレベルを決める。ここは決め打ちで書いてる
func (c *constGame) nextLevel() int {
if c.clear < 10 {
return 1
}
if c.clear < 20 {
return 2
}
return 3
}

func (c *constGame) isCorrect(word string) bool {
return c.currentWord == word
}
79 changes: 79 additions & 0 deletions kadai3-1/src/tomoyukikobayashi/typing/questions.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
// Package typing このパッケージはタイピングゲームに関するロジックとデータを格納します
package typing

import (
"io/ioutil"
"math/rand"
"os"
"path/filepath"
"time"

yamler "gopkg.in/yaml.v2"
)

const (
wordFile = "words.yaml"
)

// Questioner 質問で使うワードを提供します
type Questioner interface {
// GetNewWord 引数で与えた難易度に対して一つランダムにワードを返します
GetNewWord(level int) string
}

type constQuestioner struct {
qs map[int][]string
}

// NewQuestioner Questionerのコンストラクタ
func NewQuestioner() (Questioner, error) {
q := &constQuestioner{
qs: map[int][]string{},
}
if err := q.loadWords(); err != nil {
return nil, err
}
return q, nil
}

func random(min int, max int) int {
rand.Seed(time.Now().UnixNano())
Copy link

Choose a reason for hiding this comment

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

rand.Seed は毎回セットする必要はないため、一度だけ呼んであげればよいです

Copy link
Author

Choose a reason for hiding this comment

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

確かに

return rand.Intn(max-min) + min
}

func (q *constQuestioner) GetNewWord(level int) string {
rand := random(1, len(q.qs[level]))
// HACK ほんとはmap okを見た方がいいけど、省略
return q.qs[level][rand]
}

// Yaml HACK 使用しているパッケージの使用でしょうがなくpublicにしている
type Yaml struct {
Copy link

Choose a reason for hiding this comment

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

QuestionContainer とかそういう感じの名前にすると良さげですね
この struct は YAML データをパースした結果を入れるものですが、保存している先は別に YAML じゃなくても問題ありません。例えば JSON とかでも構わないわけです。
この struct の本質はレベルごとの問題を格納しているものなので、それに沿った名前にするほうが可読性も上がると思います。

Copy link
Author

Choose a reason for hiding this comment

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

ありがとうございます。おっしゃる通りで、こいつが何のためにいるのか、がわかる名前じゃないとダメですね
そもそもQuestionerの所でこの構造を知っている必要もないので、名前を変えてインターフェイスとかで渡すようにしようかと

Level1 []string `yaml:"Level1,flow"`
Level2 []string `yaml:"Level2,flow"`
Level3 []string `yaml:"Level3,flow"`
}

// TOOD NewGameでinterfaceとio.Reader渡してやる方が、Questionerが汎用になる
func (q *constQuestioner) loadWords() error {
cur, _ := os.Getwd()
// yamlファイルから語彙リストを読み出す
data, err := ioutil.ReadFile(filepath.Join(cur, wordFile))
if err != nil {
return err
}

var y Yaml
err = yamler.Unmarshal([]byte(data), &y)
if err != nil {
return err
}

// TODO レベルは今の所少ないのでとりあえずベタがき
// yamlの構成工夫 or interfaceとしてレベル別に取る関数生やして動的に撮れる方が良い
q.qs[1] = y.Level1
q.qs[2] = y.Level2
q.qs[3] = y.Level3

return nil
}
17 changes: 17 additions & 0 deletions kadai3-1/words.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
Level1:
- hoge
- foo
- baz
- bar

Level2:
- difficult
- omororororo
- feiajfiejawefa
- femiejaigfeag

Level3:
- testewatawetawet
- gvewagweawegawe
- geawegawegawegawe
- gwageawegaweg