Skip to content

112. Path Sum#27

Open
fhiyo wants to merge 1 commit intomainfrom
112_path-sum
Open

112. Path Sum#27
fhiyo wants to merge 1 commit intomainfrom
112_path-sum

Conversation

@fhiyo
Copy link
Copy Markdown
Owner

@fhiyo fhiyo commented Jun 26, 2024


if not root:
return False
node_target_pairs = [(root, targetSum)]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

好みの問題のような気もしますが、ここの初期化はnode_target_pairs = [(root, 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.

すみません、これ下のコメントにありましたね。

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.

いえいえ、ありがとうございます。自分はtargetSumから引いてくやり方の方が分かりやすいと感じたので、やはり好みじゃないかな、とは思います

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.

言い回しが分かりづらいものになっていたので一応追記。

やはり好みじゃないかな、とは思います

は好みの問題だと思う、という意味です。

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

自分の場合は再帰の実装が頭のベースにあったので引く方が自然に感じました(が、好みだと思います)

node, target = node_target_pairs.pop()
if is_leaf(node) and node.val == target:
return True
if node.left:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ここでNoneかどうかの判定をするのではなく上でnode is Noneの場合にcontinueさせるのも選択肢の一つですかね。

return False
return self._has_path_sum_helper(root, targetSum)

def _has_path_sum_helper(self, node: TreeNode, target: int) -> bool:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

この関数はhasPathSum内でしか使わないと思うので内部に定義してもよい気がしました。

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

内部で定義すると、毎回、関数が呼ばれるたびに関数が生成されて、nonlocal な変数が読めますね。よしあしは色々でしょう。

Comment on lines +111 to +113
if is_leaf(node):
return total + node.val == targetSum
total += node.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.

total += node.valを先にしてから、
return total == targetSumした方が素直だと思いました。

return False
return self._has_path_sum_helper(root, targetSum)

def _has_path_sum_helper(self, node: TreeNode, target: int) -> bool:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

関数内でもずっとrestの意味で使っているように思えるので、引数名はtargetでなく最初からrestにした方がいいと思いました。

if node.right:
node_target_pairs.append((node.right, rest))
return False
```
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

全体的に良いと思いました。自分もレビューのために解き直した際は、targetSumから引いていく形で実装しました。好みの問題であることに賛成です。

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