-
Notifications
You must be signed in to change notification settings - Fork 0
127. Word Ladder #34
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 #34
Conversation
| class Solution { | ||
| public: | ||
| int ladderLength(string beginWord, string endWord, vector<string>& wordList) { | ||
| bool no_begin = std::none_of(wordList.begin(), wordList.end(), [beginWord](string s) { |
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.
[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.
このあたりの挙動、ちゃんと理解できていなかったので調べました。
ラムダを引数として渡したタイミングでコピーされるので、呼び出しごとにコピーされる心配はなさそうと結論付けました。
初期化子なしのキャプチャに対応するそれらのデータメンバは、ラムダ式が評価されるときに直接初期化されます。
https://ja.cppreference.com/w/cpp/language/lambda#Lambda_capture
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.
あ、そうですね。
呼び出しごとに関数呼び出しでコピーされているのは s のほうですか。
ここのキャプチャーは [&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.
なるほど、確かに、ありがとうございます。
sは毎回コピーされるので、参照にしたほうが良さそうですね
| bool no_end = std::none_of(wordList.begin(), wordList.end(), [endWord](string s) { | ||
| return s == endWord; | ||
| }); | ||
| if (no_end) { | ||
| return 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.
これ、現代的な書き方ですが、これは for で書いたほうが読みやすくないでしょうか。
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.
ありがとうございます。
使用するメソッドで説明的にできると思いこちらを使いましたが、ちょっと考え直します。
| string left = wordList[i]; | ||
| string right = wordList[j]; | ||
| if (!IsNextWord(left, right)) { |
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.
ここでコピーが4回走りますね。参照でよいでしょう。
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 (no_end) { | ||
| return 0; | ||
| } | ||
| vector<set<int>> adjcent_indexes = vector<set<int>>(wordList.size(), set<int>{}); |
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.
vector<set<int>> adjcent_indexes(wordList.size()); と書いたほうがシンプルだと思いました。 vector のコンストラクターの第 2 引数を省略した場合、デフォルト値で初期化される点にご注意ください。
スペルミスがあるようです。 adjcent→adjacent
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 Solution { | ||
| public: | ||
| int ladderLength(string beginWord, string endWord, vector<string>& wordList) { | ||
| bool no_begin = std::none_of(wordList.begin(), wordList.end(), [beginWord](string s) { |
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 (std::find(wordList.begin(), wordList.end(), beginWord) == wordList.end()) {
return 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.
その書き方は思いつきませんでした、ありがとうございます。
| vector<set<int>> adjcent_indexes = vector<set<int>>(wordList.size(), set<int>{}); | ||
| for (int i = 0; i < wordList.size(); ++i) { | ||
| for (int j = 0; j < wordList.size(); ++j) { | ||
| string left = wordList[i]; |
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.
コピーを減らすため、 const string& で受けたほうが良いと思います。 const auto& で受けるのもよいと思います。
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.
ありがとうございます、このあたりコピーを減らす感覚がまだきちんとできていませんでした。
| } | ||
|
|
||
| private: | ||
| bool IsNextWord(const string left, const string right) { |
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.
こちらもコピーを減らすため、 const string& で受けたほうが良いと思います。詳しくは『Effective C++ 第3版』『20項 値渡しよりconst参照渡しを使おう』をご覧ください。
https://www.maruzen-publishing.co.jp/item/b294734.html
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.
ありがとうございます、参考にします。
| return false; | ||
| } | ||
| } | ||
| if (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.
return 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.
ありがとうございます。
例外処理を意識しすぎて不要な分岐になってしまっていました。
| if (!word_to_index.contains(adjacent_word)) { | ||
| continue; | ||
| } | ||
| const int left = i; |
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.
自分は定数であることを強調したいとき以外に、ローカルの変数に const は付けません。このあたりはチームの平均的な書き方に合わせることをお勧めいたします。
| return true; | ||
| } | ||
|
|
||
| int CountDistance(int begin_index, int end_index, vector<set<int>>& adjacent_indexes) { |
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.
distanceというと、ここで求めているものから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.
ありがとうございます、確かに混乱を生む語彙でした
| return s == beginWord; | ||
| }); | ||
| if (no_begin) { | ||
| wordList.push_back(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.
C++の仕様が分かってないので自信ないのですが、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.
はい、入力を変更しています。
条件次第で1単語追加、程度であればまぁ許容するかなというのと
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.
なるほど、説明くださりありがとうございます。(ProsConsを考えられているのでよさそうです)
| @@ -0,0 +1,90 @@ | |||
| /* | |||
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 Solution {
public:
int ladderLength(string beginWord, string endWord, vector<string>& wordList) {
unordered_set<string_view> words(wordList.begin(), wordList.end());
if (!words.contains(endWord)) {
return 0;
}
queue<string_view> que({beginWord});
auto push_adjacent_words = [&](string word) {
for (int i = 0; i < word.size(); ++i) {
char orig = word[i];
for (char c = 'a'; c <= 'z'; ++c) {
word[i] = c;
auto it = words.find(word);
if (it != words.end()) {
que.push(*it);
words.erase(it);
}
}
word[i] = orig;
}
};
for (int length = 1; !que.empty(); ++length) {
for (int size = que.size(); size; --size) {
string_view word = que.front();
que.pop();
if (word == endWord) {
return length;
}
push_adjacent_words(string(word));
}
}
return 0;
}
};
https://leetcode.com/problems/word-ladder/description/
https://discord.com/channels/1084280443945353267/1183683738635346001/1199015686404571176
leetcode_kuzuhara
vector で舐める方法だと、find も erase も遅いですからねえ。それぞれ「どれくらいの時間がかかるか」適当に見積もってみませんか?
見積もる方法って分かります?
どの単語とどの単語が隣接しているか、もうちょっと速く調べることもできるかと思います。しかし、なかなか大変なので、うーん、下を参照で。
https://cs.stackexchange.com/questions/93467/data-structure-or-algorithm-for-quickly-finding-differences-between-strings
この問題は、グラフ上のダイクストラですが、距離が全部一定なので BFS になりますね。
leetcode_ahayashi
https://cs.stackexchange.com/questions/93467/data-structure-or-algorithm-for-quickly-finding-differences-between-strings 頭から半分または尻尾から半分が一致しているはずなので、それでバケットを作ってバケット内でのみ比較すればいいというやりかたもありますね。(編集距離が1であるかの確認に、頭から何文字一致していて、尻尾から何文字一致しているかを足してやればいいという方法をどっかで使ったことあります。)https://discord.com/channels/1084280443945353267/1200089668901937312/1216123084889788486
hayashi-ay/leetcode#42
https://github.com/sakupan102/arai60-practice/pull/20/files#r1667317883
sakupan102/arai60-practice#20
fhiyo/leetcode#22
kazukiii/leetcode#21
Yoshiki-Iwasa/Arai60#22
TORUS0818/leetcode#22
Ryotaro25/leetcode_first60#22
seal-azarashi/leetcode#19
goto-untrapped/Arai60#57
nittoco/leetcode#36
hroc135/leetcode#19
Yusan1234/arai60#1
tarinaihitori/leetcode#20
https://discord.com/channels/1084280443945353267/1201211204547383386/1219179255615717399
leetcode_kanda
あと、BFS をするのに node_queue num_node_in_level で数を数えて、次のレベルに行くの、少しややこしいと思っています。データの整合性が取れているということは、読んでいる人からすると全部読み終わらないと分からないからです。書いている人は分かるわけですが。つまり、一つの変数に、2つの違う種類のものを入れておいて、その境界を個数で管理しているわけですよね。