11. Container With Most Water #8
Conversation
| int current_max_area = 0; | ||
| for (int i = 0; i < heightSize; i++) | ||
| { | ||
| if ((heightSize - i) * height[i] >= current_max_area) { |
There was a problem hiding this comment.
ネストが深く、やや読みにくく感じました。条件の真偽を逆にして、 continue したほうが読みやすくなると思います。
| * [texthonda28の11番](https://github.com/thonda28/leetcode/pull/16) | ||
|
|
||
| 左右を動かしていく方法がある。 | ||
| まずは日本語で構造を整理する。 |
There was a problem hiding this comment.
念のため確認させてください。なぜこの方法で正しく動くか説明してみていただけますか？
|
Pull Request に a.out も含まれてしまっているようです。レビューに不要なため、含めないほうがよいと思います。 |
| { | ||
| int left = 0; | ||
| int right = heightSize - 1; | ||
| int current_max_area = (right - left) * min(height[left], height[right]); |
There was a problem hiding this comment.
細かいですが、 current の情報量が少ないため、
max_areaでも十分意味が伝わると思いました。
thonda28
left a comment
thonda28
left a comment
There was a problem hiding this comment.
C 言語は詳しくないのですが、返り値に () をつける流儀があるんですかね？個人的には冗長に感じたので () がないほうが見やすいと思いました。
| for (int i = 0; i < heightSize; i++) | ||
| { | ||
| if ((heightSize - i) * height[i] >= current_max_area) { | ||
| for (int j = heightSize - 1; j > i; j--) |
There was a problem hiding this comment.
全組合せを見るのであれば、よくあるシンプルな二重ループで見ていくのがわかりやすそうです。j を右端から見ていくことと if 文での判定を入れることでいくつかの area の計算をスキップできる嬉しさや途中での打ち切りができる嬉しさがあるということかと思いますが、その嬉しさよりも可読性がより優先度が高いかなと個人的には思います。
| for (int i = 0; i < heightSize; i++) | |
| { | |
| if ((heightSize - i) * height[i] >= current_max_area) { | |
| for (int j = heightSize - 1; j > i; j--) | |
| for (int i = 0; i < heightSize; i++) | |
| { | |
| for (int j = i + 1; j < heightSize; j++) |
| { | ||
| if (height[j] >= height[i]) { | ||
| current_max_area = max(current_max_area, (j - i) * height[i]); | ||
| break ; |
There was a problem hiding this comment.
ここの break は memo.md に思考が書いてあったため意味がわかりましたが、コードだけを見たときにはコメント無しでは読み手が意図を汲み取るのに苦労しそうです。（ぱっと見でなぜこちらの分岐だけ打ち切っていいのかわかりませんでした）
| { | ||
| int left = 0; | ||
| int right = heightSize - 1; | ||
| int current_max_area = (right - left) * min(height[left], height[right]); |
| while (left < right) | ||
| { | ||
| if (height[left] < height[right]) { | ||
| left++; | ||
| } | ||
| else { | ||
| right--; | ||
| } | ||
| current_max_area = max (current_max_area, (right - left) * min(height[left], height[right])); | ||
| } |
There was a problem hiding this comment.
先に left, right を移動させてから計算を行うのが個人的には違和感がありました。おそらく初期化でした計算を再度したくなかったという意図ですかね？
ループは「今回の処理、次回の準備」のような流れで自然言語で説明できるコードが個人的にはわかりやすいと感じます。
| current_max_area = max(current_max_area, (j - i) * height[i]); | ||
| break ; | ||
| } | ||
| else { |
This Problem
https://leetcode.com/problems/container-with-most-water/description/