Skip to content

127. Word Ladder#20

Open
dxxsxsxkx wants to merge 1 commit into323_number_of_connected_componentsfrom
127_word_ladder
Open

127. Word Ladder#20
dxxsxsxkx wants to merge 1 commit into323_number_of_connected_componentsfrom
127_word_ladder

Conversation

@dxxsxsxkx
Copy link
Owner

int ladderLength(std::string beginWord, std::string endWord, std::vector<std::string>& wordList) {
wordList.emplace_back(beginWord);

std::vector<std::vector<int>> adjacency_list(wordList.size());
Copy link

Choose a reason for hiding this comment

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

これは隣接行列 (adjacency matrix) そのものだと思いました。一般に知られているものであれば、同じ名前を使うと分かりやすいです。

Copy link

Choose a reason for hiding this comment

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

失礼しました 🙇 これはword -> adjacent words のリストですね。

Comment on lines +68 to +70
if (n_different_chars > 1) {
return false;
}
Copy link

Choose a reason for hiding this comment

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

問題の制約に守られているのですが、まったく同じ単語を入力すると true が返るのが気になりました。

Comment on lines +26 to +28
if (wordList[idx_current_word] == endWord) {
return level;
}
Copy link

Choose a reason for hiding this comment

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

問題の制約で守られているのですが、wordListbeginWord を入れている都合上、beginWord == endWord のときに 1 が返るのをどう考えるのか、考察してみるといいかもしれません。


## 2回目

隣接する単語のリストを作る・単語を順番に確認していく・確認した単語を記録するという3つのタスクがある。

Choose a reason for hiding this comment

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

一応、隣接する単語のリストを作らなくてもいけました。(C++にまだ不慣れなので粗があっても見逃してください 🙏 )

#include <queue>
#include <set>
#include <string>
#include <vector>


class Solution {
public:
    int ladderLength(
        const std::string& beginWord,
        const std::string& endWord,
        const std::vector<std::string>& wordList
    ) {
        std::set<std::string> visited;
        std::queue<std::string> visiting;
        visiting.push(beginWord);
        int distance = 1;
        while (!visiting.empty()) {
            size_t num_visiting = visiting.size();
            for (size_t i = 0; i < num_visiting; ++i) {
                std::string word = visiting.front();
                visiting.pop();

                if (visited.contains(word)) { continue; }
                if (word == endWord) { return distance; }

                for (const auto& next_candidate : wordList) {
                    if (CalculateHammingDistance(word, next_candidate) == 1) {
                        visiting.push(next_candidate);
                    }
                }

                visited.insert(word);
            }

            ++distance;
        }

        return 0;
    }
private:
    int CalculateHammingDistance(const std::string& word1, const std::string& word2) {
        int hamming_distance = 0;
        // word1.size() should be the same as word2.size()
        for (size_t i = 0; i < word1.size(); ++i) {
            if (word1[i] != word2[i]) {
                ++hamming_distance;
            }
        }
        return hamming_distance;
    }
};

wordListの長さをN、各wordの長さをLとした時、N個のwordを訪れ、その中で次に訪れるノードをN回ループして長さL同士の文字列のハミング距離を計算するので、constraintsから 5000 * 5000 * 10 = 2.5 * 10 ^ 8 くらいの処理が必要になり、C++で1秒かかるかどうかくらいだと思います。LeetCodeのtime limitが2秒くらいだと思うので、(setを利用している/重複pushしているのでもう少し遅くなるだろうというのもあり)方針を立てた時にちょっと怪しむくらいの実行時間かもしれませんね。

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(std::string beginWord, std::string endWord, std::vector<std::string>& wordList) {
wordList.emplace_back(beginWord);
Copy link

Choose a reason for hiding this comment

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

すでにご存じでしたら申し訳ありません。 emplace_back() で要素を追加した場合、 emplace_back() の引数として渡したものを引数に取るコンストラクターが呼ばれ、値がインスタンシエイトされて、要素が追加されます。今回の場合ですと、 std::string のコピーコンストラクターが呼ばれることになると思います。このため、 push_back() で要素を追加した場合とほぼ同じことが起こると思います。
コピーコンストラクターが呼ばれるような場合に push_back() と emplace_back() のどちらを使うかについては、チームの平均的な書き方に合わせることをお勧めいたします。

public:
int ladderLength(std::string beginWord, std::string endWord, std::vector<std::string>& wordList) {
wordList.emplace_back(beginWord);
std::map<std::string, std::vector<std::string>> adjacent_words = makeAdjacentList(wordList);
Copy link

Choose a reason for hiding this comment

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

adjacent_words/word_to_adjacents と、同じ意味を表す変数で変数名がぶれている点が気になりました。個人的には word_to_adjacents または word_to_adjacent_words に統一するとよいと思います。

class Solution {
public:
int ladderLength(std::string beginWord, std::string endWord, std::vector<std::string>& wordList) {
wordList.emplace_back(beginWord);
Copy link

Choose a reason for hiding this comment

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

入力引数を関数内で変更している点が気になりました。呼び出し元から見ると、渡した値が関数内部で意図せず書き換えられると予期しない挙動につながる可能性があります。必要であればコピーを取ってから変更するか、関数コメントで「引数が変更される」ことを明記すると安心です。

また、C++ では一般的に、

  • 関数内で変更しない引数const 参照渡し、または値渡し
  • 関数内で変更する必要がある引数 → 非 const の参照渡し

というスタイルをよく見かけます。意図が明確になり、コードの可読性と安全性も高まると思います。

int level;
};

std::map<std::string, std::vector<std::string>> makeAdjacentList(
Copy link

Choose a reason for hiding this comment

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

C++ の関数名は UpperCamel にする場合もあります。この辺りは所属するチームの平均的な書き方に合わせることをお勧めいたします。

std::vector<std::vector<int>> adjacency_list(wordList.size());
makeAdjacencyList(wordList, adjacency_list);

int idx_beginWord = wordList.size() - 1;
Copy link

Choose a reason for hiding this comment

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

lower_snake と lowerCamel が混ざっている点が気になりました。 LeetCode の引数で指定されている変数を覗いて、 lower_snake に統一すると良いかもしれません。

std::vector<std::vector<int>> adjacency_list(wordList.size());
makeAdjacencyList(wordList, adjacency_list);

int idx_beginWord = wordList.size() - 1;
Copy link

Choose a reason for hiding this comment

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

こちらのコメントをご参照ください。
hemispherium/LeetCode_Arai60#10 (comment)

また、英語句として語順に違和感を感じました。 begin_word_index はいかがでしょうか?

}

private:
struct WordAndLevel {
Copy link

Choose a reason for hiding this comment

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

単語のインデックスとレベルが含まれているため、 WordIndexAndLevel としたほうが実体に近くなると思います。

std::queue<WordAndLevel> next_word_candidates;
next_word_candidates.emplace(idx_beginWord, 1);

std::vector<bool> checked_words(wordList.size());
Copy link

Choose a reason for hiding this comment

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

std::vector については以下のコメントをご参照ください。
Hurukawa2121/leetcode#17 (comment)

checked_words[idx_beginWord] = true;

while (!next_word_candidates.empty()) {
auto [current_word, level] = next_word_candidates.front();
Copy link

Choose a reason for hiding this comment

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

current_word という変数名ですと、型が std::string のように感じられます。 current_word_index とするのはいかがでしょうか?

const std::vector<std::string>& word_list,
std::vector<std::vector<int>>& adjacency_list
) {
int n_words = word_list.size();
Copy link

Choose a reason for hiding this comment

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

自分は num_words と書くことが多いです。この辺りは所属するチームの平均的な書き方に合わせることをおすすめします。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants