Conversation
| - 「beginWordがwordListに必ずしも含まれているわけではない」という条件で少し躓いた | ||
| - ローカルのテストケースで無限ループに陥った。wordに対して訪問済みマークをつけるのを忘れていたことが原因 | ||
| - 実装後、Wrong Answerが出た。パスの長さの管理が適切にできていなかったことが原因 | ||
| - 時間計算量：O(n^2) (nはwordListの要素数。本当は、beginWordの長さをmとしてO(n^2 m)だが、m <= 10なので無視できる) |
| } | ||
|
|
||
| checkedWords := make(map[string]struct{}) | ||
| wordQueue := []graphNode{{word: beginWord, level: 1}} |
There was a problem hiding this comment.
wordに加えてlevelの情報が入っているので、変数名に反映すると良いのかなと思いました。
| if w == endWord { | ||
| return first.level + 1 | ||
| } | ||
| if _, ok := checkedWords[w]; ok { |
| - https://cs.stackexchange.com/questions/93467/data-structure-or-algorithm-for-quickly-finding-differences-between-strings | ||
| - 速くなると思ったらLeetcodeの実行時間はstep1のBFSとほぼ同じ | ||
| - firstHalfToSecondHalfとsecondHalfToFirstHalfの初期化はO(n)でできており、BFSの無向グラフ初期化は0(n^2 / 2)なのでこの部分はかなり短縮できたはず | ||
| - 一方、isOneCharacterDifferent(str1, str2)とisOneCharacterDifferent(str2, str1)を計算せねばならず、これを必要としないstep1 BFSの方がこの部分が効率的になっている。GoにPythonのようなcacheデコレータがあれば簡単に改善できたはず |
There was a problem hiding this comment.
そうですね。Goで似た機能を実装しようとするとそれしかなさそうですね
step4で実装してみます！
| level int | ||
| } | ||
|
|
||
| func ladderLength(beginWord string, endWord string, wordList []string) int { |
There was a problem hiding this comment.
beginWord != endWord
という条件がついていましたが、この条件がないとこのコードでは何が置きますかね。
また、何が返ったら期待の動作でしょうか。
(こういうエッジケースは時々考えておきましょう。)
There was a problem hiding this comment.
このコードだと、beginWord -> xx -> ... -> yy -> endWordと遠回りしてたどり着けるならその時の語数が返り、遠回りしてたどり着けないなら0が返ります。
beginWord == endWordの時は直感的には1を返す実装にしたい気持ちになりますが、問題文の制約条件を守りたいなら返り値を(int, error)にして、return 0, errors.New("")とするかなと思います。この場合、ladderLength関数の頭でif beginWord == endWord { return ... }と書きます
There was a problem hiding this comment.
はい。まず、これは正解がありません。
ユースケースを想定して、どうするのがよいのか考えて下さい。
また、いろいろな方法で書いていますが、それぞれについて入力の制限を守らない入力にたいして、同じ結果が出るか出ないかも考えてみましょう。
たとえば、Python だと infinity をいれると無限ループということがあったりします。
|
|
||
| ```Go | ||
| func ladderLength(beginWord string, endWord string, wordList []string) int { | ||
| transformationGraph := initTransformationGraph(beginWord, wordList) |
| if isTransformable(wordList[i], wordList[j]) { | ||
| transformationGraph[wordList[i]] = append(transformationGraph[wordList[i]], wordList[j]) | ||
| transformationGraph[wordList[j]] = append(transformationGraph[wordList[j]], wordList[i]) |
There was a problem hiding this comment.
こことL21-23の処理がほぼ同じなので、関数化してもいい気がしますが、３行をわざわざするか難しいですね
There was a problem hiding this comment.
関数化すると、こんな感じですかね？動かしていないのでバグがあるかもしれませんが
func addEdgeToTransformationGraph(word1, word2 string, transformationGraph map[string][]string) {
transformationGraph[word1] = append(transformationGraph[word1], word2)
transformationGraph[word2] = append(transformationGraph[word2], word1)
}個人的には 2行 × 2箇所 の重複なのでわざわざ関数化しなくてもいいかな派です
| for _, w1 := range wordsInSameLevel { | ||
| w1FirstHalf, w1SecondHalf := firstHalf(w1), secondHalf(w1) | ||
| for _, w2SecondHalf := range firstHalfToSecondHalf[w1FirstHalf] { | ||
| w2 := w1FirstHalf + w2SecondHalf |
There was a problem hiding this comment.
処理が複雑なので、変数名w2だと少し意図が掴みづらかったです。
candidate := w1FirstHalf + candidateSecondHalf
などはどうでしょう？
この辺のネストがやや深めの気もします。
There was a problem hiding this comment.
なるほど、candidateいいですね！
ネストについてはどうしようもなかったというのが正直なところなのですが、何かネストを下げる方法ってあったりしますか？
There was a problem hiding this comment.
遅くなりすみません🙏
うーん、L166-176, L177- 187をappend_next_level_words_from_first_half, from_second_halfとそれぞれ関数化するとかですかね。
ただendWordの時にreturn levelしなきゃいけないので、endWordかどうかも返さないといけなくなり、それはそれでややこしいですね
| - firstHalfToSecondHalfとsecondHalfToFirstHalfの初期化はO(n)でできており、BFSの無向グラフ初期化は0(n^2 / 2)なのでこの部分はかなり短縮できたはず | ||
| - 一方、isOneCharacterDifferent(str1, str2)とisOneCharacterDifferent(str2, str1)を計算せねばならず、これを必要としないstep1 BFSの方がこの部分が効率的になっている。GoにPythonのようなcacheデコレータがあれば簡単に改善できたはず | ||
|
|
||
| ```Go |
There was a problem hiding this comment.
個人的には、
- firstHalfToSecondHalf, secondHalfToFirstHalfを尽くす処理
- 次のlevelの単語を探す処理
あたりで関数化したいです。
There was a problem hiding this comment.
そうですね、今自分でコードを読み返そうとしてすごい嫌な気分になりました、、
これは関数化必要ですね
| } | ||
|
|
||
| func firstHalf(word string) string { | ||
| return word[:(len(word)+1)/2] |
There was a problem hiding this comment.
逆にこれは関数化する必要があまりない気もしました(変数名を適切に付ければ伝わるような…)
There was a problem hiding this comment.
ここを関数化した理由としては、登場頻度が多いこともありますが、数式の入力はタイポしそうだなという不安があった、ということの方が大きいです。あとタイポして後からデバッグするときに見つけるのが大変、、
なので読み手の気持ちというより書き手の気持ちとして関数化したくなりました
| return 0 | ||
| } | ||
|
|
||
| func isTransformable(word1, word2 string) bool { |
There was a problem hiding this comment.
遷移先を探す際、
- あらかじめ文字列とそのインデックスの map を作っておく。
- 遷移元の文字列を 1 文字ずつ a～z に置き換え、 1. の map の中に含まれているかを調べる。
としたほうが速いようです。
5 participants
https://leetcode.com/problems/word-ladder/description/