#415 disjoint sets impl #421
Conversation
|
@0crat status |
|
@0crat assign @fanifieiev |
@paulodamaso This pull request #421 is assigned to @fanifieiev/z, here is why; the budget is 15 minutes, see §4; please, read §27 and when you decide to accept the changes, inform @paulodamaso/z (the architect) right in this ticket; if you decide that this PR should not be accepted ever, also inform the architect; this blog post will help you understand what is expected from a code reviewer; there will be no monetary reward for this job |
|
@fanifieiev/z this job was assigned to you 5days ago. It will be taken away from you soon, unless you close it, see §8. Read this and this, please. |
| final List<Set<Node>> result = new ArrayList<>(unvisited.size()); | ||
| while (!unvisited.isEmpty()) { | ||
| final Node node = unvisited.iterator().next(); | ||
| final Set<Node> adding = new HashSet<>(); |
There was a problem hiding this comment.
@HDouss Honestly adding variable is very confusing here, hard to catch what it is for.
Could you please rename so that others can easily understand?
| throw new UnsupportedOperationException(this.graph.toString()); | ||
| final Set<Node> unvisited = new HashSet<>(this.graph.nodes()); | ||
| final List<Set<Node>> result = new ArrayList<>(unvisited.size()); | ||
| while (!unvisited.isEmpty()) { |
There was a problem hiding this comment.
@HDouss Sorry, I could catch the whole algorithm, but is it possible to use iterator.hasNext() for the unvisited?
There was a problem hiding this comment.
@fanifieiev it is not really possible, unless to get a new iterator from the set in each iteration. This set is getting shrinked (by more than one element) in each iteration. Even the element at which the iterator is pointing is being removed from the set.
| @@ -38,7 +37,6 @@ | |||
| public final class DisjointTest { | |||
There was a problem hiding this comment.
@HDouss Could you please add one more test where the nodes in the graph are not connected at all?
|
@fanifieiev Fixed. Please review again. |
|
@paulodamaso ping |
|
@HDouss @fanifieiev Thank you guys |
|
@rultor merge |
@paulodamaso OK, I'll try to merge now. You can check the progress of the merge here |
@paulodamaso Done! FYI, the full log is here (took me 17min) |
|
@sereshqua/z please review this job completed by @fanifieiev/z, as in §30; the job will be fully closed and all payments will be made when the quality review is completed |
|
Code review was too long (9 days), architects (@paulodamaso) were penalized, see §55 |
|
@0crat quality good |
Resolving #415
Graph disjoint sets implementation.