Skip to content

139. Word Break #40

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 1 commit into
base: main
Choose a base branch
from
Open

139. Word Break #40

wants to merge 1 commit into from

Conversation

fhiyo
Copy link
Owner

@fhiyo fhiyo commented Jul 9, 2024

class Solution:
def wordBreak(self, s: str, wordDict: List[str]) -> bool:
@cache
def breakable_from(start_index: int) -> bool:
Copy link

Choose a reason for hiding this comment

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

関数名が形容詞から始まっている点に違和感を覚えました。
can_break_from()のほうが良いと思います。
また、start_indexではなく、startでもよいのかなと思いました。

Comment on lines +143 to +144
range_start = max(0, end - max_word_length)
range_end = max(0, end - min_word_length + 1)
Copy link

Choose a reason for hiding this comment

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

命名に若干違和感を覚えました。
が、代替案が出せるかと言われると微妙です。ごめんなさい...
思いついたのでいうと、left, rightか、range_left, range_rightとかです。

Choose a reason for hiding this comment

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

https://docs.python.org/3/library/stdtypes.html#range

class range(start, stop[, step])

rangeのパラメータ名は、start, stopになっていますね。rightはinclusiveのニュアンスがある気がします。

Copy link
Owner Author

Choose a reason for hiding this comment

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

命名に若干違和感を覚えました。

ふむ...代替案もあると嬉しいですが、どちらかというとなぜ違和感を覚えたかが書いてあるとありがたいです。

rangeのパラメータ名は、start, stopになっていますね。

パラメータ名に合わせるの、たしかになと思いました

Copy link

Choose a reason for hiding this comment

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

違和感の正体について考えてみました。
wordのstartになりうるものをインクリメントして探していくときに、あまり「範囲」とは考えないからかもしれないです。wordのstartの候補となるindexが、範囲をなしているというイメージがないです。
なので、range_start, range_endと言われると、何の範囲なんだろうと感じました。
こうやって感覚を言語化して整理してみると、min_start, max_startみたいなのが適切なのかなと思いました。
納得できなければ、そういう考えもあるんだなあ程度に流してもらって大丈夫です。

Copy link

Choose a reason for hiding this comment

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

(30分くらい考えてやっと、こういうことなのかな...?と言語化できた感覚なので、全然自信はないです)

Copy link
Owner Author

Choose a reason for hiding this comment

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

こちら、ありがとうございます。

なので、range_start, range_endと言われると、何の範囲なんだろうと感じました。

それはそうですね、変数を使用している箇所と近いので分かるかなとも思ってこう書いたんですが、もう少し説明的な名前にしてもいいかもと思いました。

こうやって感覚を言語化して整理してみると、min_start, max_startみたいなのが適切なのかなと思いました。

s[start:end]のendを固定して、startを一定の範囲で動かして (breakable[start]がTrueのときに) wordsの中にいるかを見ているという感覚なので、自分は範囲な気はしているのですが、とはいえmin_start, max_startの方が分かりやすい気がしました。

result = False
for word in wordDict:
if s.startswith(word, start_index):
result |= breakable(start_index + len(word))

Choose a reason for hiding this comment

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

自分だったらbreakableがTrueだったらすぐreturnしてしまうかなと思いました。

Copy link
Owner Author

Choose a reason for hiding this comment

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

たしかにそうですね。result不要ですね

min_word_length = len(min(wordDict, key=len))
max_word_length = len(max(wordDict, key=len))
words = set(wordDict)
# breakable[i]: s[:i+1] can be broken into the subset of words

Choose a reason for hiding this comment

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

s[:i]ではないですか?

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 +143 to +144
range_start = max(0, end - max_word_length)
range_end = max(0, end - min_word_length + 1)

Choose a reason for hiding this comment

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

https://docs.python.org/3/library/stdtypes.html#range

class range(start, stop[, step])

rangeのパラメータ名は、start, stopになっていますね。rightはinclusiveのニュアンスがある気がします。

breakable_indexes.add(breakable_index)
for word in wordDict:
if s.startswith(word, breakable_index):
st.append(breakable_index + len(word))
Copy link

Choose a reason for hiding this comment

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

このタイミングで breakable_indexesbreakable_index + len(word) が含まれているかどうか判定したほうが、st に不要な値を入れる必要が無くなり、速くなると思います。

if start_index == len(s):
return True
for word in wordDict:
if (s.startswith(word, start_index)

Choose a reason for hiding this comment

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

if (s.startswith(word, start_index, start_index + len(word)) のように完全一致で見た方が全体の処理回数が減りそうに思いました。

Comment on lines +194 to +206
def enumerate_prefix_words(self, s: str) -> Iterator[str]:
# enumerate the words in the Trie that are prefixes of s
node = self.root
word_as_list = []
for c in s:
if node.active:
yield ''.join(word_as_list)
if c not in node.children:
return
word_as_list.append(c)
node = node.children[c]
if node.active:
yield ''.join(word_as_list)

Choose a reason for hiding this comment

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

最後のif node.active:チェックを省くのに以下のようにするのもアリですかね。

    def enumerate_prefix_words(self, s: str) -> Iterator[str]:
        node = self.root
        word_as_list = []
        for c in s:
            if not c in node.children:
                return
            word_as_list.append(c)
            if node.children[c].is_word:
                yield ''.join(word_as_list)
            node = node.children[c]

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.

7 participants