Conversation
| for c1, c2 in zip(word1, word2): | ||
| if c1 != c2: | ||
| num_diff += 1 | ||
| if num_diff > 1: |
There was a problem hiding this comment.
num_diffをインクリメントしたのみだけ、比較すれば良いように思います。
There was a problem hiding this comment.
ありがとうございます。
for c1, c2 in zip(word1, word2):
if c1 != c2:
num_diff += 1
if num_diff > 1:こうですね。
処理の流れをちゃんと追ってなかったです。
|
|
||
| class Solution: | ||
| def ladderLength(self, beginWord: str, endWord: str, wordList: List[str]) -> int: | ||
| def is_convertible(word1: str, word2: str) -> bool: |
There was a problem hiding this comment.
transformとconvertを同じ意味で使っていますか?用語を統一した方が良いかもしれません。
There was a problem hiding this comment.
はい。
問題文で使っているので、transformが良いかなと思うんですが、is_transformableみたいな感じで違和感ないでしょうか。
| return num_diff == 1 | ||
|
|
||
| word_and_num_transformations = deque([(beginWord, 1)]) | ||
| candidate_words = set(wordList) |
There was a problem hiding this comment.
candidate_wordsから減らしていくより、使ったwordを記録する方が自然かもしれないです。
| for i in range(len(wordList)): | ||
| if is_convertible(beginWord, wordList[i]): | ||
| word_to_dist_and_adjacent_words[beginWord].append((1, wordList[i])) | ||
| word_to_dist_and_adjacent_words[wordList[i]].append((1, beginWord)) |
There was a problem hiding this comment.
私もちょっと冗長だなと思いながら書きました。
if beginWord not in word_to_dist_and_adjacent_words:この条件を満たしたら、wordListをwordList + [beginWord]と拡張して、for i & for jみたいな感じですかね。。
liquo-riceさんならどう纏めますか?
| - punctuation = !"#$%&'()*+,-./:;<=>?@[\]^_`{|}~. | ||
| - printable = digits + ascii_letters + punctuation + whitespace | ||
| - 選択肢いろいろ | ||
| - 一文字違いの候補をリストアップする方法 |
There was a problem hiding this comment.
1 <= beginWord.length <= 10
endWord.length == beginWord.length
1 <= wordList.length <= 5000
(かなり大雑把な評価)
一文字以外を全て生成する:10 * 26 * 10 = 2600
一文字以外のものを配列から探す:5000 * 10 = 50000
| return num_diff == 1 | ||
|
|
||
| candidates = deque([(beginWord, 1)]) | ||
| next_words = set(wordList) |
There was a problem hiding this comment.
next_wordsというよりは未使用の単語の集合、の方が実態と近いですかね。unused_words。
There was a problem hiding this comment.
ありがとうございます。
他の実装の命名に引っ張られてしまいました。。
|
|
||
| def get_next_words(word: str) -> list[str]: | ||
| next_words = [] | ||
| for i in range(len(word)): |
There was a problem hiding this comment.
def generate_patterns(word: str) -> Iterator[str]:
for i in range(len(word)):
yield f"{word[:i]}*{word[i+1:]}"
...
for pattern in generate_patterns(word):
...好みかもですが、こんな感じでpatternを作るのを関数化してもいいかもと思いました
There was a problem hiding this comment.
word[:i] + '*' + word[i + 1:]
これが2回出てくるのが確かに気になりますね。
|
|
||
| if is_from_begin: | ||
| words_from_begin = next_words | ||
| found_words_from_begin = found_words |
There was a problem hiding this comment.
下の found_words_from_end = found_words もそうですが、同じもの指してるから不要じゃないですか?
つまりここでは代入をしなくても found_words_from_begin is found_words がTrueな気がしました。
There was a problem hiding this comment.
ありがとうございます。不要です。
この手の無駄な処理、結構やりがちな気がしてきました。
もう少し変数間の繋がりを意識してみようと思います。
| return 0 | ||
| ``` | ||
| 思考ログ: | ||
| - なんだかんだで最後はstep1の解法で |
There was a problem hiding this comment.
Pythonだとかなりギリギリなんじゃないかと思いました(n^2 * m だと10^8を越えてくるため)。今回 m <= 10と小さいので、この制約を活かして全ての隣合うワードを生成するようにして、エッジを張り巡らしていくと O(n^2 * m) -> O(n *m^2) に落とせる気がします。
There was a problem hiding this comment.
成程、ありがとうございます。
正直まだグラフ探索に慣れておらず、余裕を持って考えられていないのだなと思いました。
ちょっと考えてみます。
| - word間の距離は全部1なので今回この情報は無駄なのだが、ダイクストラっぽくするためにこうしている | ||
| - 考え方は確か、今まで探索した最短距離をfixして、そこから辿れるノードを調査、また最短距離をfixして、、、を繰り返す感じ | ||
| - エッジの重みが非負という前提があるので上記のように最短距離を確定しながら進めることができる | ||
| - 負のエッジがある場合はワーシャルフロイドとかあった(ここら辺から記憶が朧げ) |
| words_from_begin = get_next_words(beginWord) | ||
| words_from_end = get_next_words(endWord) |
There was a problem hiding this comment.
まずは beginWord もしくは endWordから探索していくのがいいと思いました。
そうすればここのエッジ判定もなくせる気がします。
if endWord in get_next_words(beginWord):
return 2There was a problem hiding this comment.
そうですね。
これはその他の解法の実装に引っ張られている感じです。
| end_words = found_words_from_begin | ||
|
|
||
| for word in words: | ||
| found_words.add(word) |
There was a problem hiding this comment.
この位置で探索済みにするとキューに重複ノードがたくさん入ってかなり効率が落ちそうな感じがしてます。
There was a problem hiding this comment.
ちょっと書き換えてみました。これだと自分の環境で1560msです。
class Solution:
def ladderLength(self, beginWord: str, endWord: str, wordList: List[str]) -> int:
def is_convertible(word1: str, word2: str):
assert len(word1) == len(word2)
num_diff = 0
for c1, c2 in zip(word1, word2):
if c1 != c2:
num_diff += 1
if num_diff > 1:
return False
return num_diff == 1
def get_next_words(word: str) -> list[str]:
return [
next_word for next_word in wordList
if is_convertible(word, next_word)
]
if endWord not in wordList:
return 0
words_from_begin = [beginWord]
words_from_end = [endWord]
found_words_from_begin = set([beginWord])
found_words_from_end = set([endWord])
num_transformations = 1
while words_from_begin and words_from_end:
if len(words_from_begin) > len(words_from_end):
words_from_begin, words_from_end = words_from_end, words_from_begin
found_words_from_begin, found_words_from_end = found_words_from_end, found_words_from_begin
next_words = []
for word in words_from_begin:
if word in found_words_from_end:
return num_transformations
for next_word in get_next_words(word):
if next_word in found_words_from_begin: continue
found_words_from_begin.add(next_word)
next_words.append(next_word)
words_from_begin = next_words
num_transformations += 1
return 0There was a problem hiding this comment.
実装例もありがとうございます。
大変助かります!
自分でも書いてみようと思います。
| - 最近人のコードを見て、腹落ちした後に何も見ないで書いてみると、割と再現できるような感覚がある | ||
| - ここは自分なりに書きたいなあ、というところは適宜変えているが、それも”ここはこう書いてなかったなあ”という意識がある | ||
|
|
||
| パターンと文字列の対応(同値類)を作る方法 |
There was a problem hiding this comment.
ああ、これはグラフの表現として同値類を考えていたんですね。
かなりtrickyな手法に感じていたのですが、自分の中で少し抽象化できた気がします。
| return num_transformations | ||
|
|
||
| added_words = set() | ||
| for next_word in next_words: |
There was a problem hiding this comment.
word の各文字を 1 文字ずつ 'a'~'z' に書き換え、 next_words から検索する方法もあります。こちらのほうが時間計算量的に高速になると思います。パターンと文字列の対応(同値類)を作る方法も良いと思います。
RLUCacheの実装を追加
|
本問題に関連して、RLUCacheの実装を追加しました。 |
hayashi-ay
left a comment
There was a problem hiding this comment.
LRU Cacheよく書けてると思います。
ちなみに自分も書いてます。自分はPythonのLRU Chacheの実装を参考にして書きました。
https://github.com/hayashi-ay/leetcode/pull/17/files
| self.head.next = node | ||
| node.next.prev = node | ||
|
|
||
| @staticmethod |
There was a problem hiding this comment.
インスタンス化しないでこれを呼ぶような想定もないので不要ですね。(というかそういう使い方をされたら困る)
インスタンス変数にアクセスしないメソッドなのでうっかり付けてしまいました。
| self.key_to_node[key] = ListNode(key, val) | ||
| self.linked_list.add(self.key_to_node[key]) | ||
|
|
||
| if len(self.key_to_node) > self.capacity: |
There was a problem hiding this comment.
一旦last_node = self.linked_list.tail.prevのように受け取ってあげると良いかなと思いました。
There was a problem hiding this comment.
有難うございます。
選択肢にありましたが、いい名前が思い浮かばず横着してしまいました。。
| del self.key_to_node[self.linked_list.tail.prev.key] | ||
| self.linked_list.remove(self.linked_list.tail.prev) | ||
|
|
||
| def lru_cache(func): |
There was a problem hiding this comment.
サイズなども受け取れると良いかもですね。
https://docs.python.org/3/library/functools.html#functools.lru_cache
| self.head.next = self.tail | ||
| self.tail.prev = self.head | ||
|
|
||
| def add(self, node: ListNode): |
There was a problem hiding this comment.
addよりadd_front, insert_frontなどの方がより明確かもです。
There was a problem hiding this comment.
確かにDoubleLinkedListの時点ではあったほうが良かったですね。
選択肢にありましたが、その時はLRUで使うものなのでaddといえば先頭に入ると分かるだろう、と思ってしまいました。
|
実装にあたりこちらも拝見しました。 |
|
overall lgtm |
| self.head.next = self.tail | ||
| self.tail.prev = self.head | ||
|
|
||
| def add(self, node: ListNode): |
There was a problem hiding this comment.
add_first / add_front という名前にした方が良いかも
(add だと add_last を意味することが通例のため)
もしくは、move_to_head に対応させて、 add_to_head?
| self.head.next = node | ||
| node.next.prev = node | ||
|
|
||
| @staticmethod |
There was a problem hiding this comment.
内部実装上は self を使わないのですが、
クラスの外部から見たときに add などと異なり remove だけ staticmethod なのは違和感があるので、普通に使わない self を取ったメンバー関数で良いと思います。
| self.key_to_node = {} | ||
| self.linked_list = DoubleLinkedList() | ||
|
|
||
| def __str__(self): |
There was a problem hiding this comment.
この実装は linked_list の詳細を知りすぎているように感じます。
関数の中身を linked_list の __str__ に移動させてしまって、
この関数自体は linked_list.__str__() を呼ぶだけにしてしまうなどどうでしょう。
There was a problem hiding this comment.
有難うございます。
この辺の感覚がまだまだ足りないので参考になります。
| node = node.next | ||
| return ' <-> '.join(items) | ||
|
|
||
| def get(self, key) -> Optional[int]: |
There was a problem hiding this comment.
value の型を int に固定せずに Generics にしてみてはいかがでしょうか (keyも)
There was a problem hiding this comment.
そうですね。ここは限定しすぎていて用途と合っていませんでした。
| self.linked_list.move_to_head(node) | ||
| return node.val | ||
|
|
||
| def put(self, key, val) -> None: |
There was a problem hiding this comment.
Optional[ValueType]と上書きされる前の値を返してみるのはいかがでしょうか?
There was a problem hiding this comment.
なるほど、こういった返り値の設計って一般的なんでしょうか。
There was a problem hiding this comment.
Python の dict ってどうなってるっけと見たら None を返してるのですね。。
恐らくpython の dict.update は複数の値を同時に更新可能になっているからだと思います。
https://docs.python.org/3/library/stdtypes.html#dict.update
None でも良いのですが情報量が増えるので、古い値の情報を返すのは割と一般的だと思っています。
Java: https://docs.oracle.com/javase/jp/8/docs/api/java/util/Map.html#put-K-V-
Rust: https://doc.rust-lang.org/std/collections/struct.HashMap.html#method.insert
Kotlin: https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.collections/-mutable-map/put.html
C++ だと例外安全の観点から更新前値が返せないけど、古い値があったか新規かを bool で返してます。
C++: https://cpprefjp.github.io/reference/map/map/insert_or_assign.html
There was a problem hiding this comment.
詳細に有難うございます。なるほどです。
あまり深く検討してNoneを返していたわけではないので、もう少し気を遣おうと思います。
| self.linked_list.add(self.key_to_node[key]) | ||
|
|
||
| if len(self.key_to_node) > self.capacity: | ||
| del self.key_to_node[self.linked_list.tail.prev.key] |
There was a problem hiding this comment.
linked_list.tail はダミーノードでデータを持っていないという、 DoubleLinkedList の内部実装を他のクラスが知らないといけないのは辛いので、
DoubleLinkedList.get_back() などを定義するのはいかがでしょうか? (front <-> back, first <-> last)
There was a problem hiding this comment.
この感覚もあまりありませんでした。有難うございます。
確かにメソッドを定義すると他所から使いやすくなりますね。
| if key in self.key_to_node: | ||
| self.key_to_node[key].val = val | ||
| self.linked_list.move_to_head(self.key_to_node[key]) | ||
| return None |
| - ```for c1, c2 in zip(s1, s2):```これ危ない | ||
| - 問題文から```s1 == s2```が保証されているが、もし仮に長さが違ったとしたらどうなるか | ||
| - これは長さが違ってもエラーが出ず普通に動いてしまうので結構問題がある | ||
| - アサーションチェックくらいはした方がいいな、くらいの認識を持つ |
There was a problem hiding this comment.
今この問題やっててたまたま拝見したのですが、
一応version3.10からはstrict=Trueをつければ文字列が等しいチェックはしてくれるそうですね
https://docs.python.org/ja/3/library/functions.html#zip
https://leetcode.com/problems/word-ladder/description/