-
Notifications
You must be signed in to change notification settings - Fork 16
bfs #17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
bfs #17
Conversation
owenong1
commented
Jul 17, 2023
- bfs
- bfs tests
- helper classes
- minor tweaks to various names, directories
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, great job and great test cases!
But here's some suggestions that would better improve. These suggestions apply to you only because of the versatile use of BFS and DFS in so many contexts. So i was thinking perhaps you can split them up.
eg a bfs folder where within has a python file for levelOrder and another file for friendsHops. So just split them up as demonstration for bfs. Same for dfs as well with the different traversals and a general graph.
Actually, it might also be nice to have one maze/searching set-up that that dfs, bfs can be applied on. This set-up can be extended to include positive weights and apply dijkstra on in the future. But this i leave it up to you, esp if you have other plans!
public static int friendHops(GraphNode<String> personA, GraphNode<String> personB) { | ||
// Hashset to store the people we have seen already | ||
HashSet<GraphNode> checked = new HashSet<>(); | ||
// Hashmap to remember how many hops were needed to get to a specific friend * |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great comment, can also link to hashmap implementation by chang xian once up
return traversal; | ||
} | ||
|
||
// Finds the number of friend hops needed to go from person A to person B |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great explanation. But for standardisation, we use // as in-line comments to birefly explain logic or parts of the code that aren't clear. If you're explaining high-level overview eg what the method does, can use the /** */ or see how cx and kt did it!
} | ||
|
||
@Test | ||
public void bfs_friendHops_shouldReturnAccurate() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very good test cases!
* | ||
* Space: O(V): We utilize a Hashset to store the vertices we have visited already. In the worst case we have a | ||
* connected graph where all vertices are traversed, and our Hashset stores all O(V) vertices. | ||
* Further, we use a queue to hold the vertices to be traversed. In the worst case, the root will have all |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, but this isn't the only worst case. You can just mention the queue can possibly hold up to v-1 nodes, for instance when the root is connected to all other v-1 nodes.
* other vertices as neighbours, and our queue will contain O(V) vertices. | ||
* | ||
* ** Note: The above description assumes an adjacency list in order to consider neighbours in an efficient manner | ||
* If an adjacency matrix were used, it would cost O(V) to find neighbours for a single vertex, making our |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not just average! Best as well. If we have a connected graph of v nodes, we will surely 'explore' all of them. Since we do not have an efficient way of querying and getting their neighbors instantly, we have no choice but to do O(v) search of all other nodes to check if it is a neighbor of this node.
* - Push all neighbours to the queue if they have not been visited yet | ||
* - Update any variables as needed | ||
* | ||
* Time: O(V + E), where V is the number of vertices/nodes, and E is the number of edges in the graph |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interestingly, do you know when its strictly (or close to maybe 1-off) to V+E? follow-up: would a complete graph take O(V+E)?
public static List<Integer> levelOrder(BinaryTreeNode root) { | ||
if (root == null) { return new ArrayList<>(); } | ||
List<Integer> traversal = new ArrayList<>(); | ||
Queue<BinaryTreeNode> queue = new LinkedList<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can also mention in a general BFS algorithm, if the order at which the nodes to be visited at a certain level does not matter, a stack/array/arraylist/list actually suffice. The queue here is to specifically maintain the left to right order required by binary tree level-order traversal