-
Notifications
You must be signed in to change notification settings - Fork 0
127. Word Ladder #20
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
127. Word Ladder #20
Conversation
まあ、n * k + n * k + .... = nklogk と安全側に倒して考えることもできるか | ||
|
||
https://discord.com/channels/1084280443945353267/1201211204547383386/1215702126945112094 | ||
名前空間と考えるのいいな。自分は文脈という解釈をしていたけど、空間という方がしっくりくる。 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
名前空間は、名前とオブジェクトの対応を意味する専門用語です。
https://docs.python.org/ja/3.12/tutorial/classes.html#python-scopes-and-namespaces
for key in self.to_key(word): | ||
self.key_to_neighbors[key].append(word) | ||
|
||
def get_neighbors(self, word: str) -> Iterator[str]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
neighbor
として入力word
が重複して出力されるため(例: *og, d*g, do* のすべてが "dog" にマッチする)、入力 word 自身は返さないようにスキップした方が呼び出し側にとって扱いやすい気がします。
def __init__(self) -> None: | ||
self.key_to_neighbors = defaultdict(list) | ||
|
||
def to_key(self, word: str) -> Iterator[tuple]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Iterator[tuple[str, str]]
の方がぱっと見でreturn typeがわかって嬉しいですね。
def is_words_differed_by_single_letter(word1: str, word2: str): | ||
i1 = 0 | ||
i2 = 0 | ||
different_letter_count = 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
この変数を見て、ハミング距離という単語を思い出しました。
|
||
class WordNeighbors: | ||
def __init__(self): | ||
self.key_to_neighbors = defaultdict(list) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
外部から参照しない変数や呼び出さないメソッドは _ を先頭につけることが多いです。self._key_to_neighbors や self._to_key などです。
https://docs.python.org/3/tutorial/classes.html#private-variables
def __init__(self): | ||
self.key_to_neighbors = defaultdict(list) | ||
|
||
def to_key(self, word: str) -> Iterator[str]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ジェネレータなので from collections.abc import Generator として
-> Generator[tuple[str, str]] でしょうか。
https://docs.python.org/3/library/typing.html#annotating-generators-and-coroutines
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
少し悩んだのですが、
Simple generators that only ever yield values can also be annotated as having a return type of either
という記述があったので、Iteratorを選択しました
SendTypeやReturnTypeがある場合はGeneratorが良いと思いました
https://docs.python.org/3/library/typing.html#:~:text=simple%20generators%20that%20only%20ever%20yield%20values%20can%20also%20be%20annotated%20as%20having%20a%20return%20type%20of%20either "Simple generators that only ever yield values can also be annotated as having a return type of either"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
その記述は知りませんでした。Iterator でよさそうです。ありがとうございます。
if endWord not in wordList: | ||
return 0 | ||
|
||
all_words = wordList |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
呼び出し元の変数も書き換えられそうです。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ありがとうございます
FAQにも記載がありました
変数はオブジェクトへの参照でしかないことを確認しました
https://docs.python.org/ja/3.10/faq/programming.html#why-did-changing-list-y-also-change-list-x
if word == begin_word: | ||
return word_list | ||
else: | ||
return word_list + [begin_word] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
else は不要でしょうか。
以下の書き方もありだと思いました。
if begin_word in word_list:
return word_list
return word_list + [begin_word]
|
||
word_neighbors = WordNeighbors() | ||
for word in all_words: | ||
word_neighbors.add(word) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
以下のように WordNeighbors の機能にしても良いと思いました。
class WordNeighbors:
def __init__(self, words) -> None:
self.key_to_neighbors = defaultdict(list)
for word in words:
self.add(word)
word_neighbors = WordNeighbors(all_words)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
今回だとそのほうが良さそうですね!
逐次的にwordが与えられうる場合は元の実装が良さそうです
def get_all_patterns(word: str) -> list[str]: | ||
patterns = [] | ||
for i in range(len(word)): | ||
patterns.append(word[:i] + '*' + word[(i + 1):]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
なるほど、ありがとうございます!
:は評価優先度が最も低いんですね
However, in a slice the colon acts like a binary operator, and should have equal amounts on either side (treating it as the operator with the lowest priority)
変数名で悩むこともあるので空間を踏まえた命名をしたい。 | ||
|
||
いったん動くコード | ||
visitedに追加してからword_and_level_to_visitに追加しているのが気持ち悪い |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
個人的には、読んでいて違和感はありませんでした。
if adjacent_word in visited: | ||
continue | ||
visited.add(adjacent_word) | ||
adjacent_words_to_visit.add(adjacent_word) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
一度 adjacent_words に集めているんですね。
自分ならそのまま word_and_level_to_visit に入れます。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
たしかにそのままword_and_level_to_visitに入れてよいですね
visited = set() | ||
visited.add(beginWord) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
visited = set([beginWord]) で一行になります。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
visited = {beginWord}
でいきませんか。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
いけますね。
下の記事を過去に見て、{}が古いPythonで使えないと言われていたので、なんとなく避けておこうという感覚でした。
https://stackoverflow.com/questions/17373161/use-curly-braces-to-initialize-a-set-in-python
``` | ||
|
||
https://github.com/ryosuketc/leetcode_arai60/pull/20/files | ||
levelじゃなくdistance, lengthを使ってる。levelでもそんなに違和感ないかな?ただ最終的にlevelをendWordへの距離や長さとして使ってるので、lengthとかがいいかな |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
level というのは内部実装 (tree) の話なので、関数の趣旨からすると (ハミング) 距離的な naming の方がよいように感じます。
word_to_adjacent = defaultdict(list) | ||
all_word_list = wordList + [beginWord, endWord] | ||
for i in range(len(all_word_list)): | ||
for j in range(len(all_word_list)): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
枝が二重に登録されているように見えます。 for j in range(i):
としてはいかがでしょうか?
different_letter_count = 0 | ||
while i1 < len(word1) and i2 < len(word2): | ||
if word1[i1] != word2[i2]: | ||
different_letter_count += 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
different_letter_count が 1 を超えた時点で False を返すと、少し速くなるかもしれません。
word_to_adjacent[all_word_list[i]].append(all_word_list[j]) | ||
word_to_adjacent[all_word_list[j]].append(all_word_list[i]) | ||
|
||
MAX_SHORTEST_SEQUENCE_LENGTH = 2 << 30 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
十分大きな数であれば良いのですが 2 << 30
の 2
という定数に違和感を感じました。人によっては、 2
の部分に疑問を持ち、認知負荷が上がってしまうかもしれません。 1 << 30
で良いと思います。
if word == endWord: | ||
return level | ||
|
||
adjacent_words_to_visit = set() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
重複がないため list() で良いと思います。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
setだと重複を気にしているような印象を与えてしまいますね
ありがとうございます
for word in all_words: | ||
word_neighbors.add(word) | ||
|
||
queue = deque([(beginWord, 1)]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
frontier/explored という変数名もありだと思います。
問題:127. Word Ladder
次の問題:104. Maximum Depth of Binary Tree