Conversation
| func isValidBST(root *TreeNode) bool { | ||
| return validate(root, NodeRange{ | ||
| lower: math.MinInt, | ||
| upper: math.MaxInt, |
There was a problem hiding this comment.
TreeNode.Val の制約が
-2^31 <= Node.val <= 2^31 - 1
なので、32-bit環境の math.MaxInt == 2^31 - 1だとContains(境界を含まない)を誤判定しそうだなと思いました。
NullIntなどを用いて明確に値がある状態とない状態を区別した方が環境・制約が変わった時も比較的安全かなと思います。LeetCodeは64-bit環境のようですし、考えすぎな気はしますが。
There was a problem hiding this comment.
自分も同じ感想を持ちました。*int にして null 判定した方が安全だと思いました。
There was a problem hiding this comment.
お二人ともありがとうございます。
今回の場合では、まずは int64 として明示することを検討すると考えました。
公開パッケージなどでは *int で定義するのが安全ですね。
勉強になりました。
| - cpp | ||
| - シンプルなデータ構造 => 処理速度が速くメモリ使用量が少なくなる可能性がある | ||
| - TraverseInorder が何をしているのかわからなかった | ||
| - inorder でノードの値を格納していき、並び順をチェックするみたい |
There was a problem hiding this comment.
binary search tree を in-order traversal すると increasing sequence が得られるのは周りを見渡すと結構知られていることのような気がします。(私の肌感覚がずれているのかもしれません。)
There was a problem hiding this comment.
ありがとうございます。
シュッとイメージできなかったので、イメージできるようにしておきます。
| return leftIsValid && rightIsValid | ||
| } | ||
|
|
||
| type NodeRange struct { |
There was a problem hiding this comment.
ありがとうございます。
グローバルにしているという感覚はありませんでした。パッケージ構成を意識していないためだと思います。
public にしている理由は、 nodeRange と悩んだのですが、この練習会のレビュアーの方が読みやすいのでは(型として一般的なフォーマット)という理由で public にしました。
実務では private にしてパッケージ外からは使えないようにすると思います。
| leftIsValid := validate(node.Left, NodeRange{ | ||
| lower: nodeRange.lower, | ||
| upper: node.Val, | ||
| }) | ||
| rightIsValid := validate(node.Right, NodeRange{ | ||
| lower: node.Val, | ||
| upper: nodeRange.upper, | ||
| }) | ||
|
|
||
| return leftIsValid && rightIsValid |
There was a problem hiding this comment.
短絡評価されず右側のサブツリーも全部見ないと return されないので下記のように一行で書いた方が無駄な走査が減ります。
| leftIsValid := validate(node.Left, NodeRange{ | |
| lower: nodeRange.lower, | |
| upper: node.Val, | |
| }) | |
| rightIsValid := validate(node.Right, NodeRange{ | |
| lower: node.Val, | |
| upper: nodeRange.upper, | |
| }) | |
| return leftIsValid && rightIsValid | |
| return validate(node.Left, NodeRange{lower: nodeRange.lower, upper: node.Val}) && validate(node.Right, NodeRange{lower: node.Val, upper: nodeRange.upper}) |
There was a problem hiding this comment.
ありがとうございます。
ご指摘の通り、 false とわかった時点でその後の処理は不要ですね。
1行が長くなるので、書くとしたら下記のようにします。
leftIsValid := validate(node.Left, NodeRange{
lower: nodeRange.lower,
upper: node.Val,
})
if !leftIsValid {
return false
}
return validate(node.Right, NodeRange{
lower: node.Val,
upper: nodeRange.upper,
})| func isValidBST(root *TreeNode) bool { | ||
| return validate(root, NodeRange{ | ||
| lower: math.MinInt, | ||
| upper: math.MaxInt, |
There was a problem hiding this comment.
自分も同じ感想を持ちました。*int にして null 判定した方が安全だと思いました。
|
|
||
| type NodeRange struct { | ||
| lower int | ||
| upper int |
There was a problem hiding this comment.
node *TreeNode をフィールドに持たせるのもアリかなと思いました。
| } | ||
|
|
||
| func (r *NodeRange) IsBetween(v int) bool { | ||
| return r.lower < v && v < r.upper |
There was a problem hiding this comment.
lower や upper について、それらの値を含む(min, max同様に)解釈も一定ありそうです。私の感覚でもそうで「ここは等号が成り立たなくていいのかな?」と一瞬不安になりました。lowerExclusive のように明記するのが一番間違いが無さそうです。これは Contains や IsBetween などでの関数名でも同様です。
There was a problem hiding this comment.
ありがとうございます。
ここも命名に悩みました。
go だったかは忘れたのですが、何らかのライブラリでわかりやすい名前で表現していたものがあった気がするので、探してみます。
| leftNodeRange := NodeRange{start: math.MinInt, end: root.Val} | ||
| rightNodeRange := NodeRange{start: root.Val, end: math.MaxInt} |
There was a problem hiding this comment.
勘違いだったら申し訳ないのですが,例えば一つのノードからなり,root.Val == math.MinInt(= -2^31)やroot.Val == math.MaxInt(=2^32 - 1)の入力に対して正しくTrueを返せるのでしょうか?
contains()は等号なしなのでFalseにならないかなあと思いました.
There was a problem hiding this comment.
ありがとうございます。
32ビット環境だと意図した通りには動かないと思います。
go では int32 と int64 と int があり、 int は64ビット環境では int64 となります。
その前提で書いたので、 int64 と明示すべきだと考えました。
There was a problem hiding this comment.
intが環境依存なのですね,知らなかったので理解が深まりました.ありがとうございます!
| leftNodeRange := NodeRange{start: math.MinInt, end: root.Val} | ||
| rightNodeRange := NodeRange{start: root.Val, end: math.MaxInt} | ||
|
|
||
| leftIsValid := isValidBSTWithNodeRange(root.Left, leftNodeRange) | ||
| rightIsValid := isValidBSTWithNodeRange(root.Right, rightNodeRange) |
There was a problem hiding this comment.
好みの問題ですが,isValidBSTWithNodeRange(root, NodeRange{start: math.MinInt, end: math.MaxInt})とrootをそのままチェック関数に投げる方がわかりやすく感じます.
| func (r NodeRange) leftNodeRange(rootValue int) NodeRange { | ||
| return NodeRange{ | ||
| start: r.start, | ||
| end: rootValue, | ||
| } | ||
| } | ||
|
|
||
| func (r NodeRange) rightNodeRange(rootValue int) NodeRange { | ||
| return NodeRange{ | ||
| start: rootValue, | ||
| end: r.end, | ||
| } | ||
| } |
There was a problem hiding this comment.
この辺りはメソッド化しなくても良いように感じました.実際,Step2のコードの方が読みやすく感じました.
今回の問題
Validate Binary Search Tree - LeetCode
使用言語
Go
次に解く問題
Minimum Depth of Binary Tree - LeetCode