Skip to content

Conversation

@hroc135
Copy link
Owner

@hroc135 hroc135 commented Jan 14, 2025

func wordBreak(s string, wordDict []string) bool {
wordDictInitialsToWords := make(map[byte][]string)
for _, w := range wordDict {
wordDictInitialsToWords[w[0]] = append(wordDictInitialsToWords[w[0]], w)
Copy link

Choose a reason for hiding this comment

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

これ、頭文字ごとに分類してから、HasPrefix のチェックしていますがなくてもいいのではないでしょうか。
状況によっては少し速くはなりますが。

Copy link
Owner Author

Choose a reason for hiding this comment

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

たしかに思っていたよりも速くならなさそうです。平均で高々26倍ですかね

Copy link

Choose a reason for hiding this comment

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

```

#### 2c
- Trieを使う方法
Copy link

Choose a reason for hiding this comment

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

あまり時間がかかるようだったら生成AIに聞くと直してくれるでしょう。

最終的な目的は、解けるようになることで、解くことではないので、後から戻ってくるなり色々工夫してください。

Copy link
Owner Author

Choose a reason for hiding this comment

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

ありがとうございます。肩の力が抜けました

}

suffixes := []string{s} // used as a stack
checkedSuffixesMemo := make(map[string]struct{})
Copy link

Choose a reason for hiding this comment

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

checkedSuffixes でも意味が通りますし、シンプルでよいと思います。

suffixes := []string{s} // used as a stack
checkedSuffixesMemo := make(map[string]struct{})
for len(suffixes) > 0 {
top := suffixes[len(suffixes)-1]
Copy link

Choose a reason for hiding this comment

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

stack の top という意味だと思うのですが、 stack から取り出した時点ですでに top ではなくなっていると思います。 suffix が入っていると思いますので、そのまま suffix という変数名にするとよいと思います。


```Go
func wordBreak(s string, wordDict []string) bool {
canBeSegmentedHead := make([]bool, len(s)+1)
Copy link

Choose a reason for hiding this comment

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

自分なら breakableAt という変数名にすると思います。

Copy link
Owner Author

Choose a reason for hiding this comment

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

ありがとうございます。ここはいい表現が思いついていませんでした

Comment on lines +74 to +75
top := suffixes[len(suffixes)-1]
suffixes = suffixes[:len(suffixes)-1]
Copy link

Choose a reason for hiding this comment

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

Goだとpopがないようですが、ここでpop()という関数を自作しちゃった方が意図がわかりやすいような気もします

if !strings.HasPrefix(top, w) {
continue
}
topSuffix := top[len(w):]
Copy link

Choose a reason for hiding this comment

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

wordDictにある単語を除くという操作をしたことが、ちょっと読んでて時間かかってしまいました。(このまま下を読んでいくと、topSuffixがなんなのか途中で忘れて、わからなくなってくる)
個人的にはsuffixAfterRemoveとか、afterWordRemovalとかの方がわかりやすいかなと思いました。

Copy link
Owner Author

Choose a reason for hiding this comment

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

ご指摘ありがとうございます。自分で読み返してたしかにそうだと思いました。


canBeSegmented := make([]bool, len(s)+1)
for i := 0; i < len(s); i++ {
if i != 0 && canBeSegmented[i] == false {
Copy link

Choose a reason for hiding this comment

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

ループの外側でi = 0の時Trueにした方がわかりやすい気がします

for _, child := range node.children {
if child.value == word[i] {
node = child
goto wordLoop
Copy link

Choose a reason for hiding this comment

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

gotoはちょっとわかりにくいと思います。(foundとかのbool変数で管理した方がいいのかな)

goto wordLoop
}
}
node.children = append(node.children, &TrieNode{word[i], []*TrieNode{}})
Copy link

Choose a reason for hiding this comment

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

childが見つからず新たに作った後もnodeをchildに移動しないと、正しく構築できない気がしますがどうでしょう

Copy link
Owner Author

Choose a reason for hiding this comment

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

はい、こちらのコードは、wordDictからTrie木を作る -> 出来上がったTrie木にsと合致するパスがあるかどうかを調べる、ということしかしていなかったです。Trie木を実装することに精一杯で問題を解くためにどう使ったら良いかが見えておらずでした

trieRoot := InitTrie(wordDict)

var backtrack func(s string) bool
backtrack = func(s string) bool {
Copy link

Choose a reason for hiding this comment

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

backtrackという関数名は、dfsなどと同様、ややわかりにくく感じました

continue
}
topSuffix := top[len(w):]
if len(topSuffix) == 0 {
Copy link

Choose a reason for hiding this comment

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

個人的には空文字列もstackに突っ込んでpop()した後にTrueの判定をした方が、Trueの条件が読んでいて早期にわかって安心感がある、かつfor文の中身が少なくなって見やすくなって好きですが、好みの範囲そうです。

if canSplitAtIndex(index + len(word)) {
return true
}
memo[index+len(word)] = false
Copy link

Choose a reason for hiding this comment

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

この行必要なさそうでしょうか

Copy link
Owner Author

Choose a reason for hiding this comment

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

本当ですね。自分もレビュー時にこういうのに気づけるようになりたいです


```Go
func wordBreak(s string, wordDict []string) bool {
memo := make(map[int]bool) // falseのものだけmemoに追加
Copy link

Choose a reason for hiding this comment

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

trueを追加しなかったのはどのような意図がありますか?

Copy link

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants