Skip to content

82. remove duplicates from sorted list ii#4

Open
maeken4 wants to merge 6 commits into
mainfrom
82.-Remove-Duplicates-from-Sorted-List-II
Open

82. remove duplicates from sorted list ii#4
maeken4 wants to merge 6 commits into
mainfrom
82.-Remove-Duplicates-from-Sorted-List-II

Conversation

@maeken4
Copy link
Copy Markdown
Owner

@maeken4 maeken4 commented May 24, 2025



# step3
一重目のwhileで注目しているノードをcur_nodeで置いたけどあんまりわかりやすくなっていない?
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

まあ、いいと思います。ただ、cur_node の cur はあんまり読む役に立っていない気はします。
https://docs.google.com/document/d/11HV35ADPo9QxJOpJQ24FcZvtvioli770WWdZZDaLOfg/edit?tab=t.0#heading=h.xzxd7jwvkwc5
コメント集のここらへんですかね。

完走した人から気に入った人を探しておくと、毎回安定するかと思います。

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.

ありがとうございます、私もここでcur_nodeを置くのはあまり有効でなく感じます。なかなか人が覚えられないのですが意識して見てみます。

Copy link
Copy Markdown

@ryosuketc ryosuketc left a comment

Choose a reason for hiding this comment

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

LGTM です!

ListNode* deleteDuplicates(ListNode* head) {
ListNode dummy(0, head);
ListNode* tail = &dummy;
while (tail->next && tail->next->next) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

好みかとは思うのですが、個人的には cur_node (node) で回すほうが読みやすいかなと感じます (node が、メインの動かす対象で、tail はそれに付随して動くイメージなので)。

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.

ありがとうございます!ryosukeさんのコードを参考に書いてみました。確定済みのノードと作業中のノードの意図が伝わりやすくていいですね。

class Solution {
   public:
    ListNode* deleteDuplicates(ListNode* head) {
        ListNode dummy(-1, head);
        ListNode* last_non_duplicate_node = &dummy;
        ListNode* cur_node = last_non_duplicate_node->next;
        while (cur_node && cur_node->next) {
            if (cur_node->val != cur_node->next->val) {
                last_non_duplicate_node = cur_node;
                cur_node = cur_node->next;
                continue;
            }
            int val_to_remove = cur_node->val;
            while (cur_node && cur_node->val == val_to_remove) {
                ListNode* to_delete = cur_node;
                cur_node = cur_node->next;
                delete to_delete;
            }
            last_non_duplicate_node->next = cur_node;
        }
        return dummy.next;
    }
};

}
return dummy.next;
}
};
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:
ListNode* deleteDuplicates(ListNode* head) {
// スタック上に動的確保することで開放しなくてよくなる
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nit: 解放

while (tail->next && tail->next->val == val_to_remove) {
ListNode* toDelete = tail->next;
tail->next = tail->next->next;
delete toDelete;
Copy link
Copy Markdown

@nodchip nodchip May 26, 2025

Choose a reason for hiding this comment

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

ここで delete してよいか微妙に思いました。

仮に呼び出す側の立場に立って考えた場合、 ListNode* を引数として渡す関数がある場合、渡すインスタンスを呼び出し側で保持し続けるべきか、関数の中で解放されるのかが気になると思います。このあたりは関数の仕様として、関数のコメントに書かれていて欲しく思います。

仮に前者だった場合、関数の中で delete すると、二重解放になり、異常終了すると思います。後者だった場合、渡されたすべてのインスタンスを解放しないと、メモリリークとなります。 delete してよいかどうかは、関数の仕様次第だと思いました。

なお、ここで delete して異常終了しないということは、 LeetCode がこのプログラムを動かす場合は、引数として渡したメモリは delete せず、プロセスの終了時に OS に解放を任せるという実装になっているのだと思います。

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.

@nodchip
お返事遅くなりすみません。
この問題や前後の問題でもその点は迷ったところでした。コメントでどちらの立場か明示的にしようと思います。
当問題については、もし重複したノードを削除していなかった場合入力と戻り値を見比べて不要になったノードだけ削除する手間が生じる可能性があるかなと思いました。
一般に参照で受け渡しをする場合、どちらでメモリの解放をするべきなのかはよくわかっていないです…

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

一般に参照で受け渡しをする場合、どちらでメモリの解放をするべきなのかはよくわかっていないです…

念のため確認させてください。 C++ にはポインターと参照があります。この違いについて説明してみてください。

上記の質問はひとまず置いておいて、「参照」で受け渡しをする場合、関数の外側がインスタンスの所有権を持つことがほとんどだと思います。その場合、外側で解放することになります。

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.

それぞれ次のように考えています。

  • ポインタ:インスタンスの値が入っているアドレスの番地。
int a = 5;
int* p = &a; // ポインタpはa=5が格納されている番地(0x11111111みたいな)
&p = 10; // aの値は10になる
  • 参照
    CにはなくC++からある機能。変数のエイリアスを作成する。
int b = 10;
int& c = b;
std::cout << c << std::endl;  // 10

func(int& x) {
    x += 10;
}
func(b); // bは20になる

たしかにポインタと参照がごちゃごちゃになっていました。
関数に「参照」を渡す場合、エイリアスを渡しているに過ぎないので、呼び出し元で解放するのが自然ですね。

上記の質問はひとまず置いておいて、「参照」で受け渡しをする場合、関数の外側がインスタンスの所有権を持つことがほとんどだと思います。その場合、外側で解放することになります。

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

実務では、ポインターの代わりに std::unique_ptr や std::shared_ptr といったスマートポインターを使用する場合があります。
std::unique_ptr の場合、 std::unique_ptr のインスタンスを保持している一人が所有権を持っているとみなします。そして、保持している者が std::unique_ptr を解放する責任を負います。
std::shared_ptr の場合、 std::shared_ptr のインスタンスを保持している複数人が所有権を持っているとみなします。そして、全員が std::shared_ptr を解放する責任を負います。また、所有権は持たないが、参照だけしたい者は、 std::weak_ptr を保持することになります。 std::weak_ptr を保持している者は、 lock() を呼び出し、 std::shared_ptr を取得し、 std::shared_ptr 経由で参照することになります。
ドキュメントに一通り目を通しておくことをお勧めします。

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.

ありがとうございます。ドキュメント読んでみます。今の問題であれば、ListNodeの定義のnextをstd::unique_ptr型に置き換えて、親ノードが子ノードへのポインタの所有権を唯一持っているという形にするとよさそうです。

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.

5 participants