Skip to content

Create No3.21. Merge Two Sorted Lists.md#3

Open
kzhra wants to merge 1 commit into
mainfrom
kzhra-patch-1
Open

Create No3.21. Merge Two Sorted Lists.md#3
kzhra wants to merge 1 commit into
mainfrom
kzhra-patch-1

Conversation

@kzhra

@kzhra kzhra commented May 10, 2024

Copy link
Copy Markdown
Owner

No description provided.

@kzhra

kzhra commented May 10, 2024

Copy link
Copy Markdown
Owner Author

return list1;
}
if(list1->val > list2->val) {
swap(list1, list2);

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* mergeTwoLists(ListNode* list1, ListNode* list2) {
ListNode dummy_node(0);

@colorbox colorbox May 11, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

dummy_nodeという名前よりも、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.

私はこの名前はどちらも趣味の範囲かなと思います。このノードは、再帰構造を変形したために形式的に必要となったもので返り値には関わらないものであるという意図を強く出すと dummy と呼びたくなります。

ListNode* tail = &dummy_node;

while(list1 && list2) {
if(list1->val < list2->val) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

list1->val < list2->valの関係性をswapを利用して、ループの先頭で維持することで、似たようなことをしているelse節が不要になって読みやすくなりそうです。
また、その際、list1,list2という変数名ではなく、smaller,biggerという変数名にするとそのような関係性であることを変数名から明示できてわかりやすくできます。

@colorbox

Copy link
Copy Markdown

これはコードに対するコメントではなく、PRの作成に関するコメントですが
マークダウンファイルに全ステップのコードをまとめて書くのではなく、ステップごとにファイルを分けるとレビューしやすくなりそうです

@liquo-rice

Copy link
Copy Markdown

インデント直してください。

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