Skip to content

Conversation

@argondev22
Copy link
Owner

Status

Accepted

Time taken

28 m 23 s

…ing a two-pointer technique to efficiently remove specified values from the array.
@argondev22 argondev22 requested a review from Copilot October 23, 2025 22:49
@argondev22 argondev22 self-assigned this Oct 23, 2025
@argondev22 argondev22 linked an issue Oct 23, 2025 that may be closed by this pull request
@argondev22 argondev22 moved this to Doing in My Learnings Oct 23, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

あなたの解答を評価させていただきます!

📊 総合評価:6点 / 10点

良い点(+6点)

1. 正しく動作する(+2点)

// テストケース
nums = [3, 2, 2, 3], val = 3
// 結果: k = 2, nums = [2, 2, -1, -1] ✓
  • ロジックは正確で、期待通りの結果が得られます
  • In-place修正の要件を満たしています

2. 後ろから処理する発想(+2点)

let i = nums.length - 1;  // 後ろから走査
let j = nums.length - 1;  // 後ろから詰める
  • 前から処理するより効率的な場面があるという発想は良い
  • 配列の末尾から処理することで、特定のケースで上書きを避けられる

3. 2ポインタ手法を使っている(+1点)

  • i(走査用)とj(境界用)の2つのポインタを使用
  • 基本的なアプローチは理解している

4. 変数名が明確(+1点)

let i = nums.length - 1;  // 走査位置
let j = nums.length - 1;  // valでない要素の末尾

改善が必要な点(-4点)

1. 不要な複雑さ(-2点)

問題点:removedNum = -1を使う理由がない

const removedNum = -1;  // ❌ 不要
nums[j] = removedNum;   // ❌ k以降は何でもいいのに書き換えている

問題文より:

"The remaining elements of nums are not important"
(numsの残りの要素は重要ではない)

k以降の要素は見られないので、わざわざ-1に書き換える必要はありません。

// 具体例
入力: nums = [3, 2, 2, 3], val = 3

// あなたのコード
nums = [2, 2, -1, -1]  // -1に書き換えている(不要な処理)
       └k=2 └─無視─┘

// 実は元のままでOK
nums = [2, 2, 2, 3]    // これでも正解
       └k=2 └無視┘

2. 条件分岐が冗長(-1点)

if (nums[i] === val) {
    if (i === j) {  // ❌ この分岐は不要
        nums[j] = removedNum;
    } else {
        nums[i] = nums[j];
        nums[j] = removedNum;
    }
    j--;
}

i === jのケースも同じ処理で済む:

if (nums[i] === val) {
    nums[i] = nums[j];  // i === j でも問題ない(自分を自分に代入)
    j--;
}

3. 制約への配慮不足(-1点)

問題の制約を確認:

  • 0 <= nums[i] <= 50
  • 0 <= val <= 100

あなたのコードではremovedNum = -1を使っていますが:

// もし val = -1 だったら?(制約上ありえないが)
nums = [1, 2, -1, 3], val = -1
// -1に置き換えると混乱する可能性

// より良い方法:書き換えない

制約を見ると-1は使われないのでたまたま動いているだけです。

🎯 最適解との比較

シンプルな前から処理する方法(推奨)

function removeElement(nums: number[], val: number): number {
    let k = 0;
    for (let i = 0; i < nums.length; i++) {
        if (nums[i] !== val) {
            nums[k] = nums[i];
            k++;
        }
    }
    return k;
}

メリット:

  • ✅ シンプルで分かりやすい
  • ✅ 不要な処理がない
  • ✅ 時間計算量 O(n)、空間計算量 O(1)

あなたのアプローチの改善版

function removeElement(nums: number[], val: number): number {
    let i = 0;
    let j = nums.length - 1;
    
    while (i <= j) {
        if (nums[i] === val) {
            nums[i] = nums[j];  // 後ろの要素で上書き
            j--;
        } else {
            i++;
        }
    }
    
    return j + 1;
}

このアプローチのメリット:

  • valの要素が多い場合、書き込み回数が少ない
  • 例: [1,1,1,1,2], val=1 → 1回の書き込みで済む

📊 計算量の比較

解法 時間計算量 空間計算量 書き込み回数
あなたの解答 O(n) O(1) valの個数分の不要書き込み
前から処理 O(n) O(1) 最小限
改善版(後ろから) O(n) O(1) 最小限

🔧 あなたのコードの具体的な改善

改善版1:不要な処理を削除

export function removeElement(nums: number[], val: number): number {
    let i = nums.length - 1;
    let j = nums.length - 1;

    while (i >= 0) {
        if (nums[i] === val) {
            // removedNumへの書き換えを削除
            nums[i] = nums[j];  // これだけでOK
            j--;
        }
        i--;
    }

    return j + 1;
}

改善版2:より効率的に

export function removeElement(nums: number[], val: number): number {
    let i = 0;
    let j = nums.length - 1;
    
    while (i <= j) {
        if (nums[i] === val) {
            nums[i] = nums[j];
            j--;
            // i++ しない(入れ替えた要素をチェックする必要があるため)
        } else {
            i++;
        }
    }
    
    return j + 1;
}

💡 学習ポイント

1. 問題の要件を正確に理解する

"The remaining elements are not important"
→ k以降は触らなくてOK!

2. 不要な処理を避ける

// ❌ 見られない部分を書き換える
nums[j] = removedNum;

// ✅ 何もしない(効率的)
// そのまま

3. エッジケースをテストする

// i === j のケース
nums = [2], val = 2
// あなたのコードは特別扱いしているが、実は不要

📝 まとめ

良かった点

  • ✅ 基本的なロジックは正しい
  • ✅ 2ポインタ手法を理解している
  • ✅ 後ろから処理するアイデアは良い

改善点

  • removedNum = -1は不要(過剰な処理)
  • ❌ 条件分岐が冗長
  • ❌ 問題要件を過剰に満たそうとしている

次のステップ

  1. 問題文の「重要でない」部分を見極める
  2. シンプルな解法から始める
  3. 最適化は必要な時だけ

全体的には理解度は高いですが、シンプルさを意識するともっと良くなります! 🚀

Comment on lines +2 to +20
const removedNum = -1;
let i = nums.length - 1;
let j = nums.length - 1;

while (i >= 0) {
if (nums[i] === val) {
if (i === j) {
nums[j] = removedNum;
} else {
nums[i] = nums[j];
nums[j] = removedNum;
}
j--;
}
i--;
}

return j + 1;
};
Copy link

Copilot AI Oct 23, 2025

Choose a reason for hiding this comment

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

The sentinel value -1 may conflict with actual array values. LeetCode's problem constraint allows values from 0 to 50, but negative numbers are valid in general arrays. The problem doesn't require marking removed elements—simply moving them to the end is sufficient.

Suggested change
const removedNum = -1;
let i = nums.length - 1;
let j = nums.length - 1;
while (i >= 0) {
if (nums[i] === val) {
if (i === j) {
nums[j] = removedNum;
} else {
nums[i] = nums[j];
nums[j] = removedNum;
}
j--;
}
i--;
}
return j + 1;
};
let k = 0;
for (let i = 0; i < nums.length; i++) {
if (nums[i] !== val) {
nums[k] = nums[i];
k++;
}
}
return k;
}

Copilot uses AI. Check for mistakes.
Comment on lines +8 to +13
if (i === j) {
nums[j] = removedNum;
} else {
nums[i] = nums[j];
nums[j] = removedNum;
}
Copy link

Copilot AI Oct 23, 2025

Choose a reason for hiding this comment

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

The conditional check i === j is unnecessary. When i equals j, nums[i] = nums[j] is effectively a no-op assignment of the same value, and the assignment nums[j] = removedNum can be done unconditionally. This logic can be simplified to remove the conditional branch.

Suggested change
if (i === j) {
nums[j] = removedNum;
} else {
nums[i] = nums[j];
nums[j] = removedNum;
}
nums[i] = nums[j];
nums[j] = removedNum;

Copilot uses AI. Check for mistakes.
}

return j + 1;
};
Copy link

Copilot AI Oct 23, 2025

Choose a reason for hiding this comment

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

[nitpick] Remove the semicolon after the closing brace. Function declarations don't require trailing semicolons in TypeScript/JavaScript.

Suggested change
};
}

Copilot uses AI. Check for mistakes.
@argondev22 argondev22 added this to the First Time milestone Oct 23, 2025
…specifying that all feedback and comments must be provided in Japanese while allowing code examples to remain in their original languages.
@argondev22 argondev22 requested a review from Copilot October 23, 2025 22:53
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@argondev22 argondev22 merged commit f56da01 into main Oct 23, 2025
@github-project-automation github-project-automation bot moved this from Doing to Done in My Learnings Oct 23, 2025
@argondev22 argondev22 deleted the 202051024-28-remove-element branch October 23, 2025 22:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

28. Remove Element

2 participants