Skip to content

Conversation

@rossy0213
Copy link
Owner

// Time spend: 04:55
class Solution {
public int numIslands(char[][] grid) {
if (grid == null || grid.length == 0) {
Copy link

Choose a reason for hiding this comment

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

grid == null であることや grid.length == 0 であることは、入力の条件からあり得ません。そのためこの if 文は削除してよいと思います。

Copy link
Owner Author

Choose a reason for hiding this comment

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

確かに、削除します。

int cnt = 0;
for (int y = 0; y < grid.length; y++) {
for (int x = 0; x < grid[0].length; x++) {
if (grid[y][x] == '1') {
Copy link

Choose a reason for hiding this comment

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

この分量であれば大きな問題はないのですが、

if (grid[y][x] != '1') {
    continue;
}

として、 early return したほうが読みやすいかもしれません。

early return とは、本筋に関係ない例外処理等を、できるだけ早い段階で行い、 break・continue・return 等で打ち切り、本筋を下のほうにまとめて書くという書き方です。このような書き方をすることによって、本筋に関係ない処理をワーキングメモリーから追い出し、認知負荷を下げ、最短で正確にコードを理解させることができると思います。

return 0;
}

int cnt = 0;
Copy link

Choose a reason for hiding this comment

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

count または counter の略であるということは分かります。ですが、省略形は読むにあたり認知負荷が上がる傾向があります。できる限り、変数に含まれる値がなんであるかを端的に表す英単語・英語句をフルスペルで用いたほうが良いと思います。

チーム内で、省略形について合意形成が得られている場合に限り、それらを使うとよいと思います。 number_of_ を表す num_ や、 maximum_number_of_ を表す max_ は、しばしば使用されると思います。

Copy link
Owner Author

Choose a reason for hiding this comment

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

仕事で最初に使った言語がGoで、コンテキストから読み取れるものは省略できるもの全部省略していこうっていう言語思想だったので、それが抜けてなくてたまに別の言語でやったりしてます。

countで書くようにします。

return cnt;
}

public void findBoundariesOfIsland(char[][] grid, int x, int y) {
Copy link

Choose a reason for hiding this comment

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

findBoundariesOfIsland() ですと、島の境界を探す関数と誤解されると思います。 fillIland() はいかがでしょうか?

Copy link
Owner Author

Choose a reason for hiding this comment

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

fillIslandいいですね。そうします。

}

public void findBoundariesOfIsland(char[][] grid, int x, int y) {
if (y >= 0 && y < grid.length && x >= 0 && x < grid[0].length && grid[y][x] == '1') {
Copy link

Choose a reason for hiding this comment

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

不等号の向きについては、人によって好みが分かれると思います。主に

  1. 比較される側を常に左に書く派
  2. 数直線上に並べるように、左から右に向かって小さいほうから大きいほうに並べる派

がいると思います。個人的には 2. のほうが読みやすく感じます。

また、 early return するため

if (!(0 <= y && y < grid.length && 0 <= x && x < grid[0].length)) {
    return;
}

if (grid[y][x] != '1') {
     return 
}

としたほうが読みやすいように思います。座標に関する条件と、土地かどうかの条件は、条件の意味合いが異なるため、別の if 文に独立して書きました。

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 (!(0 <= y && y < grid.length && 0 <= x && x < grid[0].length)) {
return;
}

内保される条件文全部確認してから脳内で再度Notに変換する必要があるため、複合的な条件文に対してNotをつけるのは個人的にわかりにくい気がしてました。実際どっちの方が好ましいでしょうか?

Copy link

Choose a reason for hiding this comment

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

チームの標準的な書き方に合わせるのが良いと思います。チームに所属している場合は、周囲の方に聞いて、マジョリティーな書き方をするとよいと思います。また、複数の書き方を書き分けられるようにしておき、それを示すのが良いと思います。

Copy link

Choose a reason for hiding this comment

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

これ、色々だと思いますが、
!(0 <= y && y < grid.length && 0 <= x && x < grid[0].length)
0 > y || y >= grid.length || 0 > x || x >= grid[0].length
あたりの順序は気持ちが伝わります。
あとは、関数化してしまうか。
!inside(grid, x, y)

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

@nodchip nodchip left a comment

Choose a reason for hiding this comment

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

よいと思います。

@rossy0213 rossy0213 merged commit 7ade686 into main Apr 14, 2024
@rossy0213 rossy0213 deleted the arai60-17 branch April 14, 2024 17:57
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.

3 participants