Skip to content

2. Add Two Numbers#5

Open
fhiyo wants to merge 4 commits intomainfrom
2_add-two-numbers
Open

2. Add Two Numbers#5
fhiyo wants to merge 4 commits intomainfrom
2_add-two-numbers

Conversation

@fhiyo
Copy link
Copy Markdown
Owner

@fhiyo fhiyo commented May 8, 2024

class Solution {
public:
ListNode* addTwoNumbers(ListNode* l1, ListNode* l2) {
ListNode dummy_head = ListNode();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ListNode dummy_head; のほうがシンプルだと思います。

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.

https://en.cppreference.com/w/cpp/language/default_initialization
デフォルト初期化、あまり分かってなくて避けちゃうんですよね。

if T is a (possibly cv-qualified) non-POD(until C++11) class type, the constructors are considered and subjected to overload resolution against the empty argument list. The constructor selected (which is one of the default constructors) is called to provide the initial value for the new object;

ListNodeは non POD (is_pod<ListNode>::valuefalse でした) なのでデフォルトコンストラクタが呼ばれるってことなんですかね...ただC++11までと書いてあるしPODの条件も良くわからない。
今回はコピー初期化の書き方になってましたが、大体 ListNode foo = {}; のリスト初期化を使います。こちらならどうでしょうか?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

「(until C++11)」の部分が何を表しているのか、自分も分かりませんでした。

ListNode foo = {}; はあまり見ないように思います。

Google C++ Style Guide には、明示的な言及は見つけられませんでしたが、
ListNode dummy_head;
相当の書き方が例示されているようです。
https://google.github.io/styleguide/cppguide.html#Local_Variables

C++ 得意な方、補足をお願いできますでしょうか…。

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 言語の struct はメモリーの初期化がなされないので確保が速く、そこに memcpy したりします。C++ はこれを引き継いでいるわけですが、vector などでこれをやられたら大事件です。こういったものを区別して扱わないといけません。
C++11で、POD (Plain Old Data、C 言語由来) が trivial, trivially_copyable, standard_layout などに概念が整理されたためにその注釈がついていますね。実用上はあまり変わっていなさそうです。

あとは、未定義動作ですかね。プログラムは未定義動作が含まれていたらなんでもありなので、含まれないことを仮定して最適化していいという利点があり、その境界はきちんと決めないといけません。char はおそらく文字列として使われるので、未定義動作になりにくくなっているみたいですね。

リスト初期化はあまり見慣れていないのですが、これは C++11 からの機能で、私は03くらいまでの機能で書いていたというだけのことかと思います。

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.

うーんリファレンスやGoogle C++ Style Guide見てみましたが判断が難しい...

リスト初期化はあまり見慣れていないのですが、これは C++11 からの機能で、私は03くらいまでの機能で書いていたというだけのことかと思います。

なるほどです。じゃあいったんリスト初期化でいこうかな、と思います。未初期化の変数関係の未定義動作も嫌ですし...何かご意見ありましたら後でも良いので是非。

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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はUser-Provided Constructorsを定義しているので、default-initializationが実行されたときはそちらが呼ばれる。で、そのdefault constructorの定義は ListNode() : val(0), next(nullptr) {} なので、data memberが不定になることはない。
...という感じですかね。この挙動に毎回思いを馳せるのは辛いので、記事にも

As these rules are not intuitive, the easiest rule to remember to ensure an object is initialized is to provide an initializer. This is called value-initialization and is distinct from default-initialization, which is what the compiler will perform otherwise for scalar and aggregate types.

と書いてるので明示的に初期化子をつけようと思います。あと

struct Foo {
  int v = 0;
};

のようなdefault member initializationも良さそうですね (シンプルな構造体だったらこっちかなぁ...)。

ListNode* addTwoNumbers(ListNode* l1, ListNode* l2) {
ListNode* dummy_head = new ListNode(0);
ListNode* added = dummy_head;
int curry = 0;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

curry はカレー料理だと思います。 carry だと思います。

class Solution {
public:
ListNode* addTwoNumbers(ListNode* l1, ListNode* l2) {
ListNode dummy_head = ListNode();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

0 <= Node.val <= 9という条件なので、念の為dummy_headの初期値をこれ以外(-1とか)にしておいた方が良い気がします。

}

private:
static int getValOrDefault(const ListNode* const 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.

Defaultだと何を返すのか少しわかりにくい気がしたのでgetValOfNodeなどで良い気がしましたが, どうでしょうか

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.

getValOfNodeだと引数がnullptrのときにどうなるのか迷うかもと感じました。
ただdefaultが何なのか関数名を見ただけでは分からず、かつその値がアルゴリズムにおいて重要なのはそうだなと思ったので getValOrZeroとかにした方がいいかもな、と思いました

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

getValOrZeroもいいですが、

static int getValOrDefault(const ListNode* const node, int default_val);

const int total = getValOrDefault(l1, /*default_val=*/0) + getValOrDefault(l2, /*default_val=*/0) + curry;

としてもいいかもしれません。

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.

たしかにgetOrDefaultで期待するシグネチャはそれかもですね


class Solution {
public:
ListNode* addTwoNumbers(ListNode* l1, ListNode* l2) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

帰りがけの処理がないなら、簡単にiterativeに書き直せますよね。


```cpp
enum RecursiveDirection {
go, back
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

51edcc9#diff-63ff0854b4840d6c1c97b191d3a4dc35391f9e6b3f3fd9b9bcc38c7e3d1068d2R284-R286
こんな感じですかね。しかしenum classを使うのはともかく、kのprefix付けるのはハンガリアン記法だと思うのですが、このルールに従うメリットってどういうものなのでしょうか?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

スタイルガイドには、マクロとの区別と書いてありますね。enum classならそもそもcollisionが発生しないかもしれないですが…
https://stackoverflow.com/questions/5016622/where-does-the-k-prefix-for-constants-come-from

};

struct DigitInfo {
ListNode* prev;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

もともとの帰りがけの再帰だとprevがなかったですよね。そもそもprevがあったら、簡単に行き掛けにできるはずです。

@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.

6 participants