Graph bfs dfs 695#18
Conversation
| } | ||
|
|
||
| // find land | ||
| if (element == 1 && !visited[rowIndex][columnIndex]) { |
There was a problem hiding this comment.
Step1 みたいに、探索の処理は関数に切り出した方が読みやすいと思いました
There was a problem hiding this comment.
そうですね、おっしゃる通りだと思います
| return maxAreaOfIslands; | ||
| } | ||
|
|
||
| private int countNumberOfIslands(int[][] grid, boolean[][] visited, int rowIndex, int columnIndex) { |
There was a problem hiding this comment.
この関数名だと grid 内の島の数を数えているように見えました。 countAreaOfIsland などいかがでしょう？
There was a problem hiding this comment.
そうですね、Areaを数えているのでそちらの方が適切だと思います。ありがとうございます
| int element = grid[rowIndex][columnIndex]; | ||
|
|
||
| // if element is not land or water. | ||
| if (element != 1 && element != 0) { |
There was a problem hiding this comment.
0, 1は water, land のような定数にした方が扱いやすいと思いました。
There was a problem hiding this comment.
そうすれば if (element == land) のように書くことができ、可読性があがるので // if element is land. のようなコメントを削れると思います
There was a problem hiding this comment.
そうですね、ありがとうございます
|
|
||
| for (int rowIndex = 0; rowIndex < grid.length; rowIndex++) { | ||
| for (int columnIndex = 0; columnIndex < grid[0].length; columnIndex++) { | ||
| int element = grid[rowIndex][columnIndex]; |
There was a problem hiding this comment.
element という変数名には情報が乏しいと思いました（あらゆる配列の要素に関して使えてしまう）。grid の中のセルという意味で cell はどうでしょう？
There was a problem hiding this comment.
cell ありですね、サクサクっと書いていくと命名の精度が落ちてしまうので気をつけたいです。
This reverts commit 947fb51.
| // count top | ||
| if (previousRowIndex >= 0 | ||
| && grid[previousRowIndex][currentColumnIndex] == 1 | ||
| && !visited[previousRowIndex][currentColumnIndex] | ||
| ) { | ||
| visited[previousRowIndex][currentColumnIndex] = true; | ||
| lands.add(new int[]{previousRowIndex,currentColumnIndex}); | ||
| } |
There was a problem hiding this comment.
この処理は4回くり返し書くにはちょっと長く感じました。メソッドに切り出す、長さ4の配列を作りループする、など対処法がコメント集にあったと思うので、見ていなければ見てみると参考になるかもしれません。
There was a problem hiding this comment.
ありがとうございます、他の方のPR見てみます。
hiroki-horiguchi-dev
left a comment
hiroki-horiguchi-dev
left a comment
There was a problem hiding this comment.
皆さんありがとうございます。
| int element = grid[rowIndex][columnIndex]; | ||
|
|
||
| // if element is not land or water. | ||
| if (element != 1 && element != 0) { |
There was a problem hiding this comment.
そうですね、ありがとうございます
| return maxAreaOfIslands; | ||
| } | ||
|
|
||
| private int countNumberOfIslands(int[][] grid, boolean[][] visited, int rowIndex, int columnIndex) { |
There was a problem hiding this comment.
そうですね、Areaを数えているのでそちらの方が適切だと思います。ありがとうございます
|
|
||
| for (int rowIndex = 0; rowIndex < grid.length; rowIndex++) { | ||
| for (int columnIndex = 0; columnIndex < grid[0].length; columnIndex++) { | ||
| int element = grid[rowIndex][columnIndex]; |
There was a problem hiding this comment.
cell ありですね、サクサクっと書いていくと命名の精度が落ちてしまうので気をつけたいです。
| } | ||
|
|
||
| // find land | ||
| if (element == 1 && !visited[rowIndex][columnIndex]) { |
There was a problem hiding this comment.
そうですね、おっしゃる通りだと思います
| // count top | ||
| if (previousRowIndex >= 0 | ||
| && grid[previousRowIndex][currentColumnIndex] == 1 | ||
| && !visited[previousRowIndex][currentColumnIndex] | ||
| ) { | ||
| visited[previousRowIndex][currentColumnIndex] = true; | ||
| lands.add(new int[]{previousRowIndex,currentColumnIndex}); | ||
| } |
There was a problem hiding this comment.
ありがとうございます、他の方のPR見てみます。
4 participants
695. Max Area of Islandを解きました、レビューお願いします