323 number of connected components in an undirected graph#20
323 number of connected components in an undirected graph#20kitano-kazuki wants to merge 5 commits intomainfrom
Conversation
次から所要時間を書く |
| return | ||
|
|
||
| visited = [False] * n | ||
| adj_matrix = create_adjacent_matrix() |
There was a problem hiding this comment.
動作的には問題ないのですが、何に依存しているのかを明にするために edges を引数に取るようにしたいです。
| adj_matrix = create_adjacent_matrix() | |
| adj_matrix = create_adjacent_matrix(edges) |
| adjacent_matrix[node2][node1] = True | ||
| return adjacent_matrix | ||
|
|
||
| def visit_all_connected_nodes(cur_node, adj_matrix, visited): |
There was a problem hiding this comment.
- 変数名はよほど長くない限り、省略しないほうが分かりやすいです
_all_は何か特別な強調をしたいニュアンスを受けました（が、そうではないですよね）
| def visit_all_connected_nodes(cur_node, adj_matrix, visited): | |
| def visit_connected_nodes(current_node, adjacent_matrix, visited): |
There was a problem hiding this comment.
cur_node は start_index あるいは start_node などどうでしょうか。current=現在の、というのが読み手には何なのか分かりにくそうです。
There was a problem hiding this comment.
startの方が良さそうですね。ありがとうございます！
| if self.rank[parent1] < self.rank[parent2]: | ||
| self.parents[parent1] = parent2 | ||
| return | ||
| elif self.rank[parent2] < self.rank[parent1]: | ||
| self.parents[parent2] = parent1 | ||
| return | ||
| else: | ||
| self.parents[parent2] = parent1 | ||
| self.rank[parent1] += 1 | ||
| return |
There was a problem hiding this comment.
- returnが冗長に感じました
- ランクが等しいときが一番特徴的な分岐なので、それを頭に持ってくると読みやすいと思いました
| if self.rank[parent1] < self.rank[parent2]: | |
| self.parents[parent1] = parent2 | |
| return | |
| elif self.rank[parent2] < self.rank[parent1]: | |
| self.parents[parent2] = parent1 | |
| return | |
| else: | |
| self.parents[parent2] = parent1 | |
| self.rank[parent1] += 1 | |
| return | |
| if self.rank[parent1] == self.rank[parent2]: | |
| self.parents[parent2] = parent1 | |
| self.rank[parent1] += 1 | |
| elif self.rank[parent1] < self.rank[parent2]: | |
| self.parents[parent1] = parent2 | |
| else: | |
| self.parents[parent2] = parent1 |
There was a problem hiding this comment.
アーリーリターンのほうがいいかなと思っていましたが、if文内の記述量が少ないので, 一目でわかるよう冗長なreturnはないほうが良さそうですね
There was a problem hiding this comment.
何らかの処理が続くのならearly returnは読み手を助けるのですが、このifブロックはその後で関数が終了することが見えているので、early returnするメリットが享受できないように感じました。
There was a problem hiding this comment.
おっしゃる通りだと思います！ありがとうございます！
| for neighbor_node in node_to_neighbors[node]: | ||
| if not visited[neighbor_node]: | ||
| frontier.append(neighbor_node) | ||
| return |
| parent = uf.find(i) | ||
| if parent in seen_parents: | ||
| continue | ||
| seen_parents.add(parent) |
There was a problem hiding this comment.
parent という概念は UnionFind クラスだけが知っていれば良いように思いました。UnionFind クラスが num_components を管理して、get_num_groups() のようなgetterを用意するとスッキリしそうです。
2 participants
https://neetcode.io/problems/count-connected-components/question