treat message by pointer #38
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
koron
commented
May 7, 2020
Comment on lines
-125
to
-128
| keys := make([]MessageMonthKey, 0, len(msgsMap)) | ||
| for key := range msgsMap { | ||
| keys = append(keys, key) | ||
| } |
| @@ -79,13 +102,15 @@ func (m *MessageTable) ReadLogFile(path string) error { | |||
| return nil | |||
| } | |||
|
|
|||
| var msgs []Message | |||
| var visibleMsgs []Message | |||
There was a problem hiding this comment.
少し後ろの、本当に必要になる直前で宣言するように移動した。
| } | ||
| thread.Put(msg) | ||
| } |
There was a problem hiding this comment.
mapの値がポインタならば以下の3ステップで書くのが見通しが良くなるコツ。
ここはその形式に直した。
- ローカル変数へ取り出してみる
- なければ空のを突っ込むローカル変数とmapに突っ込む
- ローカル変数のを加工する
Comment on lines
-120
to
-124
| if _, ok := m.MsgsMap[key]; !ok { | ||
| m.MsgsMap[key] = visibleMsgs | ||
| } else { | ||
| m.MsgsMap[key] = append(m.MsgsMap[key], visibleMsgs...) | ||
| } |
There was a problem hiding this comment.
ここは上記のスライス版。
- 存在しないkeyにmapはゼロ値を返す
- スライスのゼロ値はnil
- nil スライスは append() の第1引数に渡せる&適切にアロケーションしてくれる
のコンボでこうなる。
Comment on lines
-128
to
-131
| sort.SliceStable(m.MsgsMap[key], func(i, j int) bool { | ||
| // must be the same digits, so no need to convert the timestamp to a number | ||
| return m.MsgsMap[key][i].Ts < m.MsgsMap[key][j].Ts | ||
| }) |
| } | ||
|
|
||
| func (t Thread) LastReplyTime() time.Time { |
There was a problem hiding this comment.
以下レシーバーの名前を t から th へ変更。
テストを書くと t *testing.T となるので Thread のレシーバーとしては t は使えずぐんにょりするのを今のうちに回避した。
|
問題なさげなのでマージしました。 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Messageをポインタで取り扱うようにしました。これは #36 で指摘・依頼されたようにツール側でMessage以下の情報を埋める場合に値のままだと非効率&バグの温床になるのを事前に防ぎます。(例: メッセージ側には情報が入ってるがスレッドには入ってないなどなど)
その結果として以下のことが起こっています。
Messages([]*Message) の定義Sort()メソッドを生やした & 使ってる場所の見通しが良くなったMessagesMap(map[MessageMonthKey]Messages) の定義Keys()メソッドを生やした & 使ってる場所の見通しが良くなったThread関連Put()を生やした & 使ってる場所の見通しが良くなったtからthへ変更した。tのままテストを書き始めるとめんどいことになるので