Skip to content

300. Longest Increasing Subsequence#30

Open
TakayaShirai wants to merge 1 commit intomainfrom
300_longest_increasing_subsequence
Open

300. Longest Increasing Subsequence#30
TakayaShirai wants to merge 1 commit intomainfrom
300_longest_increasing_subsequence

Conversation

@TakayaShirai
Copy link
Copy Markdown
Owner

@TakayaShirai TakayaShirai self-assigned this Feb 23, 2026
var lisLengthsByTail = List<int>.filled(nums.length, 1);
var maxLisLength = 1;

for (var i = 1; i < nums.length; i++) {
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で初期化すると無駄なループが一度走るので i = 1 で初期化していると思うのですが、個人的には i = 1 で初期化されていることにより、なぜこうなっているのかを読み手に考えさせるよりはi = 0で初期化しても良いかなと思いました。

Suggested change
for (var i = 1; i < nums.length; i++) {
for (var i = 0; i < nums.length; i++) {

Comment on lines +63 to +71
final number = nums[i];

for (var j = 0; j < i; j++) {
final numToCompare = nums[j];

if (numToCompare < number) {
maxLengthsTo[i] = max(maxLengthsTo[i], maxLengthsTo[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.

nums[i]の変数名がすぐには何を意図しているかわからなかったです.この後に出てくるtailNumはわかりやすかったです.
あるいは,むしろnums[j]を変数でおかずそのままに,nums[i]numToCompareにした方が私はわかりやすいと感じました.
長く使う変数(定義が遠い)ほどより意図がわかる変数名に,すぐ使わなくなる変数はシンプルに名前をつけるのが個人的な好みです.
(このコード程度の規模ならいずれにしても十分理解できるとは思いますが)

}
}

maxLength = max(maxLengthsTo[i], maxLength);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

あくまで好みの問題ですが,maxLengthsTo[i]に当たる引数が長くなる場合に備えて

maxLength = max(maxLength, maxLengthsTo[i]);

の順にするのが好みです.

var lo = 0;
var hi = list.length;
while (lo < hi) {
final mid = (lo + hi) >> 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.

ビット演算で右シフトすると2による整数除算と一致すること,言われれば確かにそうなのですが,初めて気づきました.
実際のコンピュータの動作をより直接的に表現している点で,計算速度などいくつかメリットがありそうですね.

var minTailsByLISLength = <num>[double.negativeInfinity];

for (var num in nums) {
final minTailPosition = _lowerBound(minTailsByLISLength, num);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

minTailsByLISLengthの中ではnumをtailをして認識していなかったので,変数名にTailが入っていることで一瞬混乱しました.また,何がminなのかが理解できませんでした(なんならnumで終わる部分列で最長のものの長さを返しているのでは?).
とはいえより良い変数名を思いつくのも難しいですね.lengthOfIncreasingSequenceWithNumだと長すぎますしlengthOfISWithNumだとISがわかりにくそうですし,LengthTailNumも,初めて読むときには理解に時間がかかりそうです.

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.

レビューありがとうございます。

minTailsByLISLengthの意味としては、わかりづらいですが、ある特定の長さの subsequence において、最小の値を持つ tail を保存した配列になります。

例:
[10,9,2,5,3,7,101,18] の場合、
最終的な minTailsByLISLength = [-double.negativeInfinity, 2, 3, 18, 101]
長さが 1 の subsequence の最小の tail は 2
長さが 4 の subsequence の最小の tail は 101
といった形で、添字が subsequence の長さ、値が tail の位置にある最小値といった形です。
最小の tail を保存・更新していくことで、新しく値が入ってきた時に subsequence を伸ばせるかどうかを判断できるといった形式です。

可読性という観点としては、そもそもの解法が少しわかりづらいため、別の手法を優先的に使用するというのが良いのかなと個人的に思っています。

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

返信ありがとうございます!
なるほど,(minTail)の(Position)という意味ですね,min(TailPosition)と誤解していました.
確かにそれならしっくりきます.
理解すればとてもしっくりくる変数名なので,コメントで例を書く,あるいは文章で説明しても良いかもしれません.

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.

3 participants