Skip to content

2. Add Two Numbers#5

Merged
Ryotaro25 merged 6 commits intomainfrom
problem5
Sep 27, 2024
Merged

2. Add Two Numbers#5
Ryotaro25 merged 6 commits intomainfrom
problem5

Conversation

@Ryotaro25
Copy link
Owner

問題へのリンク
2. Add Two Numbers

問題文(プレミアムの場合)

備考

次に解く問題の予告
20. Valid Parentheses

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

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

} else if (l2 == NULL) {
return l1;
}
int temp = l1->val + l2->val;

Choose a reason for hiding this comment

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

tempという変数名の見直しをお願いします。

Copy link
Owner Author

Choose a reason for hiding this comment

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

繰り上げ部分とそうでない部分を区別しないニュアンスでnumberと付けてみました。

Choose a reason for hiding this comment

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

変数の命名についての議論がDiscord上に色々あったと思うので、探してみてください。

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
たくさん議論されておりました。step4.cppの変数名を更新しました。
リスト1とリスト2と繰り上げを合計したものの変数名はsumにしました。

ダミーノードから、ポインターを動かすために使っていたcurrent_nodeをtraverseに変更しました。nodeが入っていることは初期化の部分でわかると思ったので後ろにnodeはつけませんでした。
これだと何をしたい変数か伝わりますでしょうか?

a5eb918
メモ
・正解はないが意図が読み取れるもの
・無理に省略しないで、変数が何を表すのかわかるもの
・大文字はじまりは定数と誤解される。

確認したdiscord
https://discord.com/channels/1084280443945353267/1195700948786491403/1196058103242838157
こちらの7番
https://discord.com/channels/1084280443945353267/1196472827457589338/1196473202193481728
https://discord.com/channels/1084280443945353267/1200089668901937312/1201416402247098398
https://discord.com/channels/1084280443945353267/1201211204547383386/1202379368530186331
https://discord.com/channels/1084280443945353267/1210494002277908491/1215665412788985896

命名に関するソース
https://google.github.io/styleguide/pyguide.html
https://peps.python.org/pep-0008/#naming-conventions

Choose a reason for hiding this comment

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

nodeの方が分かりやすく感じました。traverseだと、動詞なので関数名のように聞こえます。

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
確認有難うございます。nodeに戻しました。

ListNode* node = new ListNode(temp % 10);
node->next = addTwoNumbers(l1->next, l2->next);
if (temp >= 10) {
node->next = addTwoNumbers(node->next, new ListNode(1));

Choose a reason for hiding this comment

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

繰り上げは別枠で足している形になりますかね。new ListNode(1)でメモリリークしてます。

Copy link

Choose a reason for hiding this comment

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

メモリーリークの問題あるんですけれども、LeetCode 上の制限というか、ですから仕方がないですよね。ただ、問題意識は持って欲しいですね。

Choose a reason for hiding this comment

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

l1のみがnullptrの場合は、確かにnew ListNode(1)が結果の一部になるかもしれないですね。carryをパラメータに含めたヘルパー関数を作る方が自然かと思います。

class Solution {
public:
    ListNode* addTwoNumbers(ListNode* l1, ListNode* l2) {
        if (!l1 && !l2) {
          return nullptr;
        }
        int sum = 0;
        if (l1) {
            sum += l1->val;
            l1 = l1->next;
        }
        if (l2) {
            sum += l2->val;
            l2 = l2->next;
        }
        ListNode* node = new ListNode(sum % 10);
        node->next = addTwoNumbers(l1, l2);
        if (sum >= 10) {
            auto carry_node = make_unique<ListNode>(1);
            node->next = addTwoNumbers(node->next, carry_node.get());
        }
        return 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.

私の書き方だと書き方メモリの解放が行われないのですね。make_unique知らなかったので下記の記事も合わせて読んでみました。指摘とコードの提供有難うございます。

https://www.geeksforgeeks.org/cpp-14-make_unique/
https://cpprefjp.github.io/reference/memory/make_unique.html

@@ -0,0 +1,26 @@
/**

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.

削除しました。


ListNode* dummy_node = new ListNode();
ListNode* current_node = dummy_node;
int carry_up = 0;

Choose a reason for hiding this comment

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

繰り上げはcarry、繰り下げはborrowです。
https://en.wikipedia.org/wiki/Carry_(arithmetic)

Copy link
Owner Author

Choose a reason for hiding this comment

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

carry upは運ぶでした。失礼しました。

public:
ListNode* addTwoNumbers(ListNode* l1, ListNode* l2) {
ListNode* node1 = l1;
ListNode* node2 = l2;

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.

list1とlist2に書き換えて、新たに変数に入れるのを辞めました。

}

int temp = node1->val + node2->val + carry_up;
bool isCarry = false;

Choose a reason for hiding this comment

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

has_carry

Copy link
Owner Author

Choose a reason for hiding this comment

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

有難うございます。スネークケースに統一もしました。


while (node1->next || node2->next) {
int temp = node1->val + node2->val + carry_up;
if (temp > 9) {

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.

マジックナンバーは意味が明らかなもののほうがいいので、私は10を推奨します。

Copy link
Owner Author

Choose a reason for hiding this comment

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

/ %に書き換えました。
2桁の数字であるかがより分かりやすいので10のほうがいいですね。

@Ryotaro25
Copy link
Owner Author

@liquo-rice @oda
レビュー有難うございます。
下記のコミットにstep4.cppとstep5.cppを追加しました。
それぞれ、step1をベースに指摘内容を踏まえたものと再帰をベースに指摘内容踏まえたものになります(こちらはliquo-riceさんのコードを写経したものになりますが🙇)
94f59e8

@Ryotaro25
Copy link
Owner Author

@liquo-rice @oda
追加で質問です。step3で以下のような部分がございます。
三項演算子を使ったほうがスッキリ書けると思ったのですが、何か判断基準はございますでしょうか?
if (l1) { l1_digit = l1->val; } else { l1_digit = 0; }

現職では禁止とされており、リーダブルコードでは可読性が上がる場合のみ使用可能、google style guideには記載が無かったです。

public:
ListNode* addTwoNumbers(ListNode* list1, ListNode* list2) {
if (!list1 && !list2) {
return NULL;

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.

誤ってコミット分けてしまいました。
ポインターにはnullptrを使うのですね🙇

@rihib rihib mentioned this pull request Aug 6, 2024
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