Skip to content

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions 6.graph/word-ladder/memo.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
## step1
- 今探索している単語の各文字を変え、その単語が `wordList` に存在したら探索。
- これを `beginWord` から行っていく。

- 重要なのは、例えば `hot`→`hit`→`nit` と `hot`→`not`→`nit` は区別しなくていいこと。
- つまり、 `hit` と `not` どちらの探索で `nit` を探索候補にし、単語リストから削除したとしても、全体の結果に影響しない。
- ただしBFSでないとだめ。
- DFSの場合、 `nit` から削除してしまう可能性があるから。

- 以下の処理がEBCDICでは通らない。
Copy link

Choose a reason for hiding this comment

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

私はこの環境を触ったことがないので基本的には気にしなくて構わないと思います。

```cpp
for (char next_char = 'a'; next_char <= 'z'; ++next_char)
```
- EBCDICの場合、26文字ハードコーディングするか、0~2^8の中で `std::islower` が真のものを英小文字とする。

## step2
- 他のコードを読む。

- 「変形する最小手順の探索(今回の問題)」と「次に探索できる文字の探索」は分けれる。
- つまり、二重ループをループ二つに分けれる。
- 参考:https://github.com/colorbox/leetcode/pull/34/

## step3
- 補完なしで5分半ほどで実装。
38 changes: 38 additions & 0 deletions 6.graph/word-ladder/step1.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
#include <map>
#include <queue>
#include <set>
#include <string>
#include <vector>

class Solution {
public:
int ladderLength(std::string beginWord, std::string endWord, std::vector<std::string>& wordList) {
std::set<std::string> candidate_words = std::set(wordList.begin(), wordList.end());
Copy link

Choose a reason for hiding this comment

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

std::set<std::string> candidate_words(wordList.begin(), wordList.end());

と書いたほうがシンプルだと思います。

Copy link
Owner Author

Choose a reason for hiding this comment

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

存じ上げませんでした。
次からそのように書きます。
ありがとうございます。

std::queue<std::string> words_to_explore;

Choose a reason for hiding this comment

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

好みの問題かもですが、XXX_to_YYYという命名規則は、一瞬std::mapの変数っぽく見えてしまうので、別の名前を使うと良いかもしれません。

Copy link
Owner Author

Choose a reason for hiding this comment

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

おっしゃる通りです。
少なくとも exploring_words などにすべきです。
ありがとうございます。

words_to_explore.push(beginWord);
int ladder_length = 0;
while (!words_to_explore.empty()) {
int current_candidate_size = words_to_explore.size();
ladder_length++;
for (int i = 0; i < current_candidate_size; ++i) {
Copy link

Choose a reason for hiding this comment

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

現在のキューの長さを保持し、その長さ分だけ先頭から取り出して処理するという実装は、個人的にはあまり見ません。キューに ladder_length 相当の値も一緒に入れて 1 要素ずつ処理するか、二つの配列を用意し、交互に入れ替えていく実装のほうが分かりやすいと思います。

Copy link
Owner Author

@Hurukawa2121 Hurukawa2121 Jan 16, 2025

Choose a reason for hiding this comment

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

ありがとうございます。
Editorialの実装なのですね。
確かに、自分のコードと比べると、動きが格段に把握しやすいです。
「よく見る実装に寄せる」を読みやすく観点として持ちます。

std::string current_word = words_to_explore.front();
words_to_explore.pop();
if (current_word == endWord) {
return ladder_length;
}
for (int next_index = 0; next_index < current_word.size(); ++next_index) {

Choose a reason for hiding this comment

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

next_という接頭辞が頻出しますが、分かりづらかったです。
隣接する単語関連の変数というのはわかりますが、各変数がロジックの中でどういう役割を負っているのかが不明瞭に感じます。
例えば、next_indexword_indexnext_charconverting_charnext_wordcanditdate_wordなどにするとコード内の実態と合って読みやすくなると思いました。

https://github.com/Hurukawa2121/leetcode/pull/20/files#r1912426507
でも指摘されていますが、このfor文のロジックをメソッド抽出すると読みやすくなるかもしれません。

Copy link
Owner Author

Choose a reason for hiding this comment

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

converting_char などは自分の選択肢にありませんでした。

ループに関しておっしゃる通りです。
step3で少し関数に切り分けましたが、指摘されてより切り分けれると思いました。

ありがとうございます。

for (char next_char = 'a'; next_char <= 'z'; ++next_char) {
std::string next_word = current_word;
next_word[next_index] = next_char;
if (!candidate_words.contains(next_word)) {
continue;
}
candidate_words.erase(next_word);
words_to_explore.push(next_word);
}
}
Comment on lines +23 to +33
Copy link

Choose a reason for hiding this comment

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

四重ループはさすが厳しいです。
BFS をしている部分と、ここの距離が1の言葉を列挙してみるところに分けるとか、なんらかの方法はあるように思います。

BFS の方法も、長くなると読みにくいものだと思います。これだと current_candidate_size が途中で変更されていないことを確認しないと BFS のつもりであることも分からないのです。

Copy link
Owner Author

Choose a reason for hiding this comment

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

step3で少し関数に切り分けましたが、指摘から考えて、より切り分けれることに気づきました。

少なくとも current_candidate_size を cost にすべきでした。

読み手の感覚まで教えていただきありがとうございます。

}
}
return 0; // endWord が見つからなかった場合
}
};
57 changes: 57 additions & 0 deletions 6.graph/word-ladder/step2.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
#include <map>
#include <queue>
#include <set>
#include <string>
#include <vector>

class Solution {
public:
int ladderLength(std::string beginWord, std::string endWord, std::vector<std::string>& wordList) {
std::set<std::string> candidate_words = std::set(wordList.begin(), wordList.end());
if (!candidate_words.contains(endWord)) {
return 0;
}
candidate_words.insert(beginWord);

auto adjacency_list = ComposeAdjacencyList(candidate_words);
std::queue<std::string> words_to_explore;
words_to_explore.push(beginWord);
int ladder_length = 0;
while (!words_to_explore.empty()) {
ladder_length++;
int current_candidate_size = words_to_explore.size();
for (int i = 0; i < current_candidate_size; ++i) {
std::string current_word = words_to_explore.front();
words_to_explore.pop();
for (std::string& next_word : adjacency_list[current_word]) {
if (next_word == endWord) {
return ladder_length + 1;
}
if (!candidate_words.contains(next_word)) {
continue;
}
candidate_words.erase(current_word);
words_to_explore.push(next_word);
}
}
}
return 0; // endWord に到達しなかった場合
}

private:
std::map<std::string, std::vector<std::string>> ComposeAdjacencyList(std::set<std::string>& candidate_words) {
std::map<std::string, std::vector<std::string>> adjacency_list;
for (const std::string& word : candidate_words) {
for (int index = 0; index < word.size(); ++index) {
for (char next_letter = 'a'; next_letter <= 'z'; ++next_letter) {
std::string transformed_word = word;
transformed_word[index] = next_letter;
if (candidate_words.contains(transformed_word)) {
adjacency_list[word].push_back(transformed_word);
}
}
}
}
return adjacency_list;
}
};
57 changes: 57 additions & 0 deletions 6.graph/word-ladder/step3.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
#include <map>
#include <queue>
#include <set>
#include <string>
#include <vector>

class Solution {
public:
int ladderLength(std::string beginWord, std::string endWord, std::vector<std::string>& wordList) {

Choose a reason for hiding this comment

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

この問題ではstring_viewを使えば、文字列のコピーは不要かもしれません。

Copy link
Owner Author

@Hurukawa2121 Hurukawa2121 Jan 13, 2025

Choose a reason for hiding this comment

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

確かに、単語の同士のハミング距離を見れば隣接リストが作れます。
ありがとうございます。

std::set<std::string> candidate_words = std::set(wordList.begin(), wordList.end());
if (!candidate_words.contains(endWord)) {
return 0;
}
candidate_words.insert(beginWord);

auto adjacency_list = ComposeAdjacencyList(candidate_words);
std::queue<std::string> words_to_explore;
words_to_explore.push(beginWord);
int ladder_length = 0;
while (!words_to_explore.empty()) {
ladder_length++;
int current_candidate_size = words_to_explore.size();
for (int i = 0; i < current_candidate_size; ++i) {
std::string current_word = words_to_explore.front();
words_to_explore.pop();
for (std::string& next_word : adjacency_list[current_word]) {
if (next_word == endWord) {
return ladder_length + 1;
}
if (!candidate_words.contains(next_word)) {
continue;
}
candidate_words.erase(current_word);
words_to_explore.push(next_word);
}
}
}
return 0; // endWord に到達しなかった場合
}

private:
std::map<std::string, std::vector<std::string>> ComposeAdjacencyList(std::set<std::string>& candidate_words) {
std::map<std::string, std::vector<std::string>> adjacency_list;
for (const std::string& word : candidate_words) {
for (int index = 0; index < word.size(); ++index) {
for (char next_letter = 'a'; next_letter <= 'z'; ++next_letter) {
std::string transformed_word = word;
transformed_word[index] = next_letter;
if (candidate_words.contains(transformed_word)) {
adjacency_list[word].push_back(transformed_word);
}
}
}
Comment on lines +45 to +53
Copy link

Choose a reason for hiding this comment

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

ループは2重までにしたい気持ちが私にはあって、
たとえば、この部分を lambda とかでもいいので、
void AddWordToAdjacencyList(const string& word, ... &adjacency_list);
などとしてもいいかもしれません。これはこれでと思います。

あと、Index で隣接リストを作ると速そうですね。(map をたどるときに文字列の比較が多数行われるため。)

Copy link
Owner Author

@Hurukawa2121 Hurukawa2121 Jan 14, 2025

Choose a reason for hiding this comment

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

ループは2重までにしたい気持ちが私にはあって
確かに、AddWordToAdjacencyLis のアイデアを用いてstep4を実装したら、とても読みやすくなりました。

「ループは2重まで」をルールにしたいと思います。

map をたどるときに文字列の比較が多数行われるため

これに気づきませんでした。
こちらも実装し、だいぶ早くなりました。

教えていただきありがとうございます。

}
return adjacency_list;
}
};
74 changes: 74 additions & 0 deletions 6.graph/word-ladder/step4.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
#include <map>
#include <queue>
#include <set>
#include <string>
#include <vector>

class Solution {
public:
int ladderLength(std::string beginWord, std::string endWord, std::vector<std::string>& wordList) {
std::set<std::string> candidate_words = std::set(wordList.begin(), wordList.end());

Choose a reason for hiding this comment

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

All the words in wordList are unique.

と問題文にあるので、文字列をコピーしてセットを作るのではなく、すでに確認済みのインデックスをセットに保存すると効率的でしょう。

if (!candidate_words.contains(endWord)) {
return 0;
}

candidate_words.insert(beginWord);
wordList.push_back(beginWord);
int begin_index = wordList.size() - 1;
std::queue<int> indexes_to_explore;
indexes_to_explore.push(begin_index);
std::vector<std::vector<int>> adjacency_list = ComposeAdjacencyList(wordList);

auto PushCandidateIndex = [&](int current_index) {
Copy link

Choose a reason for hiding this comment

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

candidateという語が指すものに一貫性がないように感じました。(BFSで次に調べるものなのか、今調べているものなのか)
そのせいで、L30でcandidate_words.erase(wordList[candidate_index]);という、candidateからなぜかcandidateを除くという一見したわかりにくい動作になっていると思います

Choose a reason for hiding this comment

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

確認済みのインデックスをセットに保存したほうが分かりやすいように思いました。

for (int candidate_index : adjacency_list[current_index]) {
if (wordList[candidate_index] == endWord) {
return false;
}
if (!candidate_words.contains(wordList[candidate_index])) {
continue;
}
candidate_words.erase(wordList[candidate_index]);
indexes_to_explore.push(candidate_index);
}
return true;
};

int ladder_length = 0;
while (!indexes_to_explore.empty()) {
ladder_length++;
int current_candidate_size = indexes_to_explore.size();
for (int i = 0; i < current_candidate_size; ++i) {
int current_index = indexes_to_explore.front();
indexes_to_explore.pop();
Copy link

Choose a reason for hiding this comment

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

上のodaさんやnodchipさんのコメントとも重なりますが、current_candidate_sizeがindexes_to_exploreのsizeを参照していて、それを元にループを回したあと、indexes_to_exploreがfrontされたりpopされたりすると、やや不安を感じます。

if (!PushCandidateIndex(current_index)) {
Copy link

Choose a reason for hiding this comment

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

wordList[candidate_index] == endWordという条件判定は関数内ではなく外で行い、PushCandidateIndexはindexes_to_exploreにpushすることに専念した方が、個人的にはわかりやすいと思います。

return ladder_length + 1;
}
}
}
return 0; // endWord に到達しなかった場合
}

private:
int HammingDistance(std::string& word1, std::string& word2) {
Copy link

Choose a reason for hiding this comment

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

word1とword2のsizeが等しくない場合にどうなるか考えてもいいかもしれません

int diff_count = 0;
for (int i = 0; i < word1.size(); ++i) {
if (word1[i] != word2[i]) {
diff_count++;
}
}
return diff_count;
}

std::vector<std::vector<int>> ComposeAdjacencyList(std::vector<std::string>& wordList) {
std::vector<std::vector<int>> adjacency_list(wordList.size());
for (int i = 0; i < wordList.size(); ++i) {
for (int j = i + 1; j < wordList.size(); ++j) {
if (HammingDistance(wordList[i], wordList[j]) == 1) {
adjacency_list[i].push_back(j);
adjacency_list[j].push_back(i);
}
}
}
return adjacency_list;
}
};