Skip to content

Conversation

@Ryotaro25
Copy link
Owner

問題へのリンク
https://leetcode.com/problems/reverse-linked-list/description/
問題文(プレミアムの場合)

備考

次に解く問題の予告
Kth Largest Element in a Stream

フォルダ構成
LeetCodeの問題ごとにフォルダを作成します。
フォルダ内は、step1.cpp、step2.cpp、step3.cppとmemo.mdとなります。

memo.md内に各ステップで感じたことを追記します。

Comment on lines 9 to 10
空間計算量
O(1)
Copy link

Choose a reason for hiding this comment

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

O(ノード数) だと思います

Copy link
Owner Author

Choose a reason for hiding this comment

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

@fhiyo
レビュー有難うございます。全て開始側のブラケットの場合O(n)になりますね。失礼しました。
下記にて修正しました。
fb59e1a

* ListNode(int x, ListNode *next) : val(x), next(next) {}
* };
*/
#include <stack>
Copy link

Choose a reason for hiding this comment

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

includeディレクティブのブロックの終わりの後は1行空ける方が主流だと思います。
(いまいち参考になるリンクが見つからなかった)

Choose a reason for hiding this comment

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

大体1行空いていますね。https://source.chromium.org/chromium/chromium/src/+/main:base/hash/hash.h
なおLeetCode上ではincludeは不要です。

Copy link
Owner Author

Choose a reason for hiding this comment

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

@fhiyo @liquo-rice
レビュー有難うございます。LeetCodeではヘッダーファイルの読み込みとusing namespace stdが自動的に行われているのですね。

step1に対する修正をstep4.cppに、step3に対する修正をstep5.cppに追加しました。
fb59e1a

}

stack<ListNode*> vals;
while (head) {
Copy link

Choose a reason for hiding this comment

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

15行目では head == nullptr としているので、どちらかに統一した方が良いと思います。

Copy link
Owner Author

Choose a reason for hiding this comment

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

@fhiyo
head もしくは!headに統一しました。

return head;
}

stack<ListNode*> vals;
Copy link

Choose a reason for hiding this comment

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

ListNodeへのポインタが格納されるstackの変数名としてvalsは少し分かりにくい気がします。
nodesでどうでしょうか

Copy link
Owner Author

Choose a reason for hiding this comment

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

@fhiyo
nodesに変更しました。もう少し伝わるよう考えないとですね。

Comment on lines +26 to +27
ListNode* reverse_node = new ListNode();
node = reverse_node;
Copy link

Choose a reason for hiding this comment

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

ヒープにメモリを確保する必要がなさそうです。今のコードだとreverse_nodeはメモリリークしますね

Suggested change
ListNode* reverse_node = new ListNode();
node = reverse_node;
ListNode reverse_node = ListNode();
node = &reverse_node;

Choose a reason for hiding this comment

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

参考:
automatic storage duration (stack) vs dynamic storage duration (heap)
https://en.cppreference.com/w/cpp/language/storage_duration
https://en.wikipedia.org/wiki/Automatic_variable

Copy link
Owner Author

Choose a reason for hiding this comment

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

@fhiyo @liquo-rice
指摘&&資料有難うございます。
メモリに対する意識がまだ低いようです。

質問です。
new演算子を使う場合は、deleteをつける対応でも良いのでしょうか?
もしくはheapメモリを使う必要のない場合はできるだけstackメモリを使うものでしょうか?

Copy link

Choose a reason for hiding this comment

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

もしくはheapメモリを使う必要のない場合はできるだけstackメモリを使うものでしょうか?

自分は基本こっちだと思っています。ヒープにメモリを確保するのは単にスタックポインタをずらすだけのローカル変数の定義よりもコストがかかるので。あとnewするとどこでそのメモリを解放するのかを気にするので読み手の負荷が高くなる気はします

if (head == nullptr) {
return head;
}

Copy link

Choose a reason for hiding this comment

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

この例外処理なくても結果的に通りますね。
あった方が安心感はあるのかもしれません。

Copy link

Choose a reason for hiding this comment

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

あと気にするとしたら、stack, ListNode のコンストラクタが走るので、これはあってもいいかもしれません。

while (head) としているので、if (!head) のほうが統一感があるかもしれません。

Copy link
Owner Author

Choose a reason for hiding this comment

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

@nittoco @oda
統一しました。何度か注意されているので、意識づけられるように工夫します。
step1に対する修正をstep4.cppに、step3に対する修正をstep5.cppに追加しました。
fb59e1a

while (head) {
vals.push(head);
head = head->next;
}
Copy link

Choose a reason for hiding this comment

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

個人的なものですが、headという変数が動くのは違和感があります

Copy link
Owner Author

Choose a reason for hiding this comment

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

nodeの変数を使うようにしました。

new演算子を使えばコンストラクターによって次のnodeにnullptrを設定してくれる。

## 他の解法
他には、再帰や特別な構造を使わないでポインタの付け替えだけでの対応などがあった。

Choose a reason for hiding this comment

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

別解も実装してみてください。

Copy link
Owner Author

Choose a reason for hiding this comment

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

@liquo-rice
memoに記載漏れておりすみません。
iterative.cpp内に実装しております。
LeetCodeの回答タイトルがrecursionとなっていたので再帰かなと思ったのですが中身は、ポインタの付け替えで同じものでした🙇

head = head->next;
}

ListNode* reverse_node = new ListNode();
Copy link

Choose a reason for hiding this comment

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

これはメモリーリークします。
ListNode reverse_node;
ListNode* node = &reverse_node;

Copy link
Owner Author

Choose a reason for hiding this comment

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

@oda
fhiyoさん、liquo-riceさんのお二人からも指摘頂いておりました。まだメモリに対する意識が低いです。

質問です。
new演算子を使う場合は、deleteをつける対応でも良いのでしょうか?
もしくはheapメモリを使う必要のない場合はできるだけstackメモリを使うものでしょうか?
heapを使わないといけないタイミングに遭遇したことがないのですが。

Copy link

Choose a reason for hiding this comment

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

うーむ。これ、一発レッドカード級なのですが、結構複雑な話なので調べてください。調べた結果もまとめて欲しいです。

Copy link
Owner Author

Choose a reason for hiding this comment

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

@oda @fhiyo @liquo-rice
stackとheapについて調べてみました。

前提としてメモリリークの対策として、オブジェクトは可能な限りスタックに置くべき。

参照
メモリリークに対する対応議論
https://stackoverflow.com/questions/76796/general-guidelines-to-avoid-memory-leaks-in-c

他にも下記のような問題がございました。
ヒープのデメリット面
・処理時間がstackに比べて遅い
・メモリの解法はプログラマーが行う必要がある(fhiyoさん指摘の読み手の負荷が高くなることにも繋がるのかと思いました。)
・使用するメモリのサイズが大きい
・スレッドセーフでない(ABA問題など)

参照
stackとheapの比較
https://www.geeksforgeeks.org/stack-vs-heap-memory-allocation/
スレッドセーフについて
https://docs.oracle.com/cd/E19455-01/806-5257/6je9h033e/index.html
ABA問題
https://en.wikipedia.org/wiki/ABA_problem

結論
heapを使うことによりデメリットを考慮すると、どうしても使う必要があるとき以外はstackを使う。
そもそものメモリリークの対応策としてheapにオブジェクトを置かない方がいい。

Choose a reason for hiding this comment

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

どのような場面でHeapは必要になりますか?

Copy link
Owner Author

Choose a reason for hiding this comment

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

@liquo-rice
再度独習c++を読み直してみました。Heap(new演算子)を使うタイミングは以下の二点。
・スコープから抜けた場合であっても破棄させない場合
・メモリを動的に確保したい場合

具体例が欲しかったので、改めて過去にnew演算子を使う必要がないと指摘を受けた箇所を見直しました。
別ページとなります🙇
step4.cpp
ここでのListNode dummy_head;は実際のオブジェクトを保持しているわけではなく、先頭のポインタを指し示すだけなのでnew演算子は必要ないです。
逆にそれ以降のforループないでは、heapに保存しておかないとスコープから出た場合(ここではforループ)にメモリの解放が行われるので値の参照を行うことが出来ません。

こちらには他の関数に渡したいときなどとございました。
https://stackoverflow.com/questions/5082680/when-should-i-use-the-new-operator-in-c

認識合ってますでしょうか?

Choose a reason for hiding this comment

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

オブジェクトのライフタイムをスコープと関連付けない場合については正しい理解だと思います。

メモリを動的に確保したい場合

メモリを動的に確保したいのは、具体的にはどんな時ですか?

Choose a reason for hiding this comment

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

ここでのListNode dummy_head;は実際のオブジェクトを保持しているわけではなく、先頭のポインタを指し示すだけなのでnew演算子は必要ないです。

この説明は少し怪しい気がします。「実際のオブジェクトを保持しているわけではなく」とはどういうことですか?

Copy link
Owner Author

@Ryotaro25 Ryotaro25 Jun 2, 2024

Choose a reason for hiding this comment

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

メモリを動的に確保したいのは、具体的にはどんな時ですか?

 入力値に左右される配列を扱う場合でしょうか。特段の理由がない限りコンテナクラスを使うと独習c++に記載ございました。
どういった場合に配列を使う必要があるのか調べてみました。
「下位レベルのレイヤーでは、動的確保で作成できるデータ構造は原始的な配列のみである。」と以下のページに記載ございました。これは知りませんでした。
https://ja.wikipedia.org/wiki/%E5%8B%95%E7%9A%84%E3%83%A1%E3%83%A2%E3%83%AA%E7%A2%BA%E4%BF%9D

「実際のオブジェクトを保持しているわけではなく」とはどういうことですか?

実際のオブジェクトを保持しているわけではなくというのはおかしいですね、、、dummy_headが動的にオブジェクトを追加しているのではなく、forループ内の各nodeがnew演算子を使って動的にメモリを確保しています。実際にはオブジェクトを保持していますが、それはnodeの先頭のポインターです。なかなか難しいですね。

@Ryotaro25 Ryotaro25 merged commit 0790ce7 into main May 14, 2025
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