Skip to content

Conversation

@TORUS0818
Copy link
Owner

Comment on lines +135 to +136
if not root:
return False

Choose a reason for hiding this comment

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

root がないケースをコーナーケースとして取り出さなくても、ループの1回目の処理で L.141-142 で正しく判定できそうです。

Copy link
Owner Author

Choose a reason for hiding this comment

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

ありがとうございます。
脳内トレース漏れしてました。

Comment on lines +131 to +133
def is_leaf(node: Optional[TreeNode]) -> bool:
assert node
return not node.left and not node.right

Choose a reason for hiding this comment

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

node が None であるケースは例外(Exception)ではないと思うので、シンプルに False を返すのが適切かなと思いました。引数の型は Optional[TreeNode] で None を許容しているにも関わらず、それを入れると例外が返ってくるというのはびっくりするかと。葉であるかの判定を関数化するなら以下のようなコードがよさそうです。

Suggested change
def is_leaf(node: Optional[TreeNode]) -> bool:
assert node
return not node.left and not node.right
def is_leaf(node: Optional[TreeNode]) -> bool:
return node and not node.left and not node.right

また上記を採用する場合、L.141-142 の判定も不要になりますね。(合わせて remain -= node.val でなく L.145 内で remain - node.val == 0 のように扱う必要もありますが)

Copy link
Owner Author

Choose a reason for hiding this comment

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

ご指摘ご尤もですね。
型への意識が弱いので、もう少し慎重にする必要がありそうです。

実装例もありがとうございます。

Comment on lines +48 to +49
if not root:
return False

Choose a reason for hiding this comment

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

ここも不要ですかね

Comment on lines +54 to +55
if not node:
continue
Copy link

Choose a reason for hiding this comment

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

ただの選択肢の話ですが、node.left or right がNoneでないときだけstackに積むようにすればここ不要になりますね。

Copy link
Owner Author

Choose a reason for hiding this comment

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

ありがとうございます。
今回は突っ込む方式にしました。

Comment on lines +145 to +146
if is_leaf(node) and remain == 0:
return True
Copy link

Choose a reason for hiding this comment

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

if is_leaf(node):
    return remain == 0

とすると、nodeが葉の場合に無駄にleft, rightをスタックに入れずに済む気がしました。

Copy link
Owner Author

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.

ん、ループでこれやるとまずくないですかね。
最初の葉が見つかった時に残数が0で無ければ探索打ち切っちゃいますよね?

Copy link

Choose a reason for hiding this comment

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

あっ本当ですね...失礼しました 🙏

step4追加
if not root.left and not root.right and root.val == targetSum:
return True

return self.hasPathSum(root.left, targetSum - root.val) or \

Choose a reason for hiding this comment

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

binary operatorの前で改行を入れると良さそうです。
https://peps.python.org/pep-0008/#should-a-line-break-before-or-after-a-binary-operator

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回目)
これは癖になっているので意識しないとです。ありがとうございました。

# self.right = right
class Solution:
def hasPathSum(self, root: Optional[TreeNode], targetSum: int) -> bool:
def is_leaf(node: TreeNode) -> bool:

Choose a reason for hiding this comment

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

細かいですが、個人的にはclassのprivateメソッドとして定義したいです。汎用的なためhasPathSumが呼び出されるたびにis_leafの関数オブジェクトを作成する必要はないように感じました。

Copy link
Owner Author

Choose a reason for hiding this comment

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

なるほど、確かにそうですね。
こういうご指摘を頂けるのもこの勉強会ならではですね。

if not node:
continue

remain -= node.val

Choose a reason for hiding this comment

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

細かいですが、remainって自動詞なのでremained,leftとかrestとかが適切なのかなと

Copy link
Owner Author

Choose a reason for hiding this comment

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

軽く調べたつもりだったんですけど、remainsにしないといけないんですね。
しかも不可算名詞だから相応しくないですね。。

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