Skip to content

Create 300. Longest Increasing Subsequence.md#34

Open
irohafternoon wants to merge 1 commit intomainfrom
300.-Longest-Increasing-Subsequence
Open

Create 300. Longest Increasing Subsequence.md#34
irohafternoon wants to merge 1 commit intomainfrom
300.-Longest-Increasing-Subsequence

Conversation

@irohafternoon
Copy link
Copy Markdown
Owner

//nums[i]を採用する場合
if (nums[j] < nums[i]) {
LIS[i][i] = std::max(LIS[i][i], LIS[i - 1][j] + 1);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

今回は、40行目の処理が37行目に影響を与えないので問題ないですが、continueしている方が自然な気がしました。

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

ありがとうございます。
コメントいただいて、nums[j] と nums[i] の大小関係に関わらず常に処理するnums[i]を採用しないケースを先に書く方法もあると思いました。

#include <algorithm>
#include <vector>

class Solution {
public:
    int lengthOfLIS(vector<int>& nums) {
        //LIS[i][j] = numsのi番目までの文字を使った時、subsequenceの最後がnumsのj番目の時のLIS 
        // i <= j の時 LIS[i][j] >= 1が保証される
        std::vector<std::vector<int>> LIS(nums.size(), std::vector<int>(nums.size()));
        for (int i = 0; i < nums.size(); i++) {
            for (int j = 0; j <= i; j++) {
                LIS[i][j] = 1;
            }
        }
        for (int i = 0; i < nums.size(); i++) {
            for (int j = 0; j < i; j++) {
                //nums[i]を採用しない場合
                LIS[i][j] = std::max(LIS[i][j], LIS[i - 1][j]);
                //nums[i]を採用する場合
                if (nums[j] < nums[i]) {
                    LIS[i][i] = std::max(LIS[i][i], LIS[i - 1][j] + 1);
                }
            }
        }
        return *std::max_element(LIS.back().begin(), LIS.back().end());
    }
};

@Fuminiton さんがイメージされているのは

#include <algorithm>
#include <vector>

class Solution {
public:
    int lengthOfLIS(vector<int>& nums) {
        //LIS[i][j] = numsのi番目までの文字を使った時、subsequenceの最後がnumsのj番目の時のLIS 
        // i <= j の時 LIS[i][j] >= 1が保証される
        std::vector<std::vector<int>> LIS(nums.size(), std::vector<int>(nums.size()));
        for (int i = 0; i < nums.size(); i++) {
            for (int j = 0; j <= i; j++) {
                LIS[i][j] = 1;
            }
        }
        for (int i = 0; i < nums.size(); i++) {
            for (int j = 0; j < i; j++) {
                //nums[i]を採用しない場合
                LIS[i][j] = std::max(LIS[i][j], LIS[i - 1][j]);
                //nums[i]を採用する場合
                if (nums[i] <= nums[j]) {
                    continue;
                }
                LIS[i][i] = std::max(LIS[i][i], LIS[i - 1][j] + 1);
            }
        }
        return *std::max_element(LIS.back().begin(), LIS.back().end());
    }
};

このようなコードでしょうか

Copy link
Copy Markdown

@Fuminiton Fuminiton May 5, 2025

Choose a reason for hiding this comment

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

すみません。言葉を端折りすぎ&コードを誤読していました。
採用する/しない場合とコメントがあったので、

if A:
    # Aの時の処理
else:
    # Aでない時の処理

もしくは

if A:
    # Aの時の処理
    continue;
# Aでない時の処理

のように独立した処理が行われているのかと読んでいて思った次第です。

一方、自分がコードを正しく解読できておらず、単にコメントをみて判断して指摘してしまっていて申し訳ないのですが、nums[i]を採用する場合にもLIS[i][j] = std::max(LIS[i][j], LIS[i - 1][j]);が必要なのですね。

なので、コードを変えるというよりは、コメントを「nums[i]を採用する場合」から「nums[i]を採用できる場合」に変え、「採用しない場合」は「共通の処理」などに変えるのはどうでしょうかというのが指摘したかったことです。

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

コメントありがとうございます。
こちらこそ、コメントの内容が誤解を招きやすいものでした。指摘いただきありがとうございます。
確かにnums[i]を採用できる場合、と共通の処理という分け方なので、この書き方は良くないと思いました

//LIS[i] = subsequenceの最後がnumsのi番目の時のLIS 最低1は保証される
std::vector<int> LIS(nums.size(), 1);
for (int i = 0; i < nums.size(); i++) {
for (int j = 0; j < i; j++) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

好みの範囲だと思いますが、jはbefore_iなどとして、iより前の値をみている意図を伝わるようにしてもいいと思います。

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

ありがとうございます。
確かにこちらはコードの長さは短いですが、indexが2つあるので、2つの関係を明示する名前をつけた方が良さそうです

}
return LIS_elements.size();
}
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

いいと思います。

for (int j = 0; j < i; j++) {
//nums[i]を採用する場合
if (nums[j] < nums[i]) {
LIS[i][i] = std::max(LIS[i][i], LIS[i - 1][j] + 1);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

i == 0 だと -1 にアクセスしていませんか?

Copy link
Copy Markdown
Owner Author

@irohafternoon irohafternoon May 5, 2025

Choose a reason for hiding this comment

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

ありがとうございます。
i == 0 の時は、j < i を常に満たさず、内側のループはスキップされるので、アクセスは起きないと理解しています。
ただ、コメントいただいたように読み手に心配をさせてしまうので、i = 1 から始めるべきと感じました

};
```

ニ分探索をする方法(番兵INT_MAXを使用して場合分けをなくす)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

これは工夫ですね。

class Solution {
public:
int lengthOfLIS(vector<int>& nums) {
std::vector<int> LIS_elements;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

コンパイラの挙動は変わらないのでいいのかもしれませんが、一応Google style guideには以下のように記載されています。

The names of variables (including function parameters) and data members are snake_case (all lowercase, with underscores between words).

https://google.github.io/styleguide/cppguide.html#Variable_Names

Goのように最初の文字が大文字か小文字かでスコープの変わる言語もあるのできになってしまいました。

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

ありがとうございます
略語は大文字の方が馴染みがあるかと思ったのですが、styleguideにも略語の変数を小文字で書いていました。

std::string fqdn = ...; // Well-known abbreviation for Fully Qualified Domain Name

lis_elementsのようにするべきでした。
Goは全く知らないのですが、大文字か小文字かでpublicかprivateかが変わるのですね。勉強になります

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