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
Added Breadth First Search [Java] #190
Conversation
README.md
Outdated
@@ -11,6 +11,7 @@ Community (college) maintained list of Algorithms and Data Structures implementa | |||
|
|||
| Algorithm | C | CPP | Java | Python | Golang | | |||
|:--------------|:----------------:|:----------------:|:----------------:|:-----------------:|:-----------------:| | |||
| [Breadth First Traversal](https://en.wikipedia.org/wiki/Breadth-first_search) | | | [\[X\]](breadth_first_traversal/BreadthFirstTraversal.java) | | | |
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.
This entry will come after Binary Search
addEdge(2, 1); | ||
addEdge(2, 3); | ||
addEdge(3, 3); | ||
System.out.println("Breadth First Traversal starting from vertex 0"); |
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.
What would this function be searching for?
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.
@singhpratyush not searching. It would give you the path of BFT from root 0 to all node.
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.
But since we are searching for things, there should be a target node.
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.
@singhpratyush Is that okay?
@aviaryan Please can check why travis-ci fail? |
System.out.println("Breadth First Traversal starting from source to destination"); | ||
breadthFirstTraversal(1,3); | ||
} | ||
} |
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.
What would be the output of this program?
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.
It gives the BFT path from source to destination.
@jsroyal Build fixed now. It was a coala problem, nothing wrong with your code. |
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.
Pleasse see comments.
@@ -0,0 +1,64 @@ | |||
import java.util.HashSet; |
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.
Inconsistent file and class names.
while (i.hasNext()) { | ||
int n = i.next(); | ||
if (n == destination) { | ||
System.out.println(n); |
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.
Do not print anything in algorithm function.
adj[source].add(w); | ||
} | ||
|
||
private static void breadthFirstTraversal(int source, int destination) { |
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.
How will other classes use this if it is private?
public class BreadthFirstSearch { | ||
|
||
// Array of lists for Adjacency List Representation | ||
public static LinkedList<Integer>[] adj; |
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.
Why bound to int
? This can be generic.
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.
@singhpratyush Please, can you explain a little bit more?
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 are saying that the adjacency list may contain only integer items, while they can be anything. They can be names, addresses, tweets, a state of N-Queen board, etc. You just need to define that they are somehow linked.
So, instead of using Integer
, you can go with generic objects.
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.
Thanks.
how should I go about? Can please write some code snippet?
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.
Use templates.
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.
Can please write some code snippet? I'm not getting, using the template in linked list.
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.
@jsroyal For templates, refer to https://github.com/iiitv/algos/blob/master/linked_list/LinkedList.java for an example.
@@ -2,7 +2,7 @@ | |||
|
|||
import java.util.*; | |||
|
|||
public class breadthFirstTraversal { | |||
public class BreadthFirstTraversal { |
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.
Better change the names to BreadthFirstSearch
instead of BreadthFirstTraversal
.
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.
Please see comments
README.md
Outdated
@@ -12,6 +12,7 @@ Community (college) maintained list of Algorithms and Data Structures implementa | |||
| Algorithm | C | CPP | Java | Python | Golang | | |||
|:--------------|:----------------:|:----------------:|:----------------:|:-----------------:|:-----------------:| | |||
| [Binary Search](https://en.wikipedia.org/wiki/Binary_search_algorithm) | [\[X\]](binary_search/binary_search.c) | | [\[X\]](binary_search/BinarySearch.java) | [\[X\]](binary_search/binary_search.py) | | | |||
| [Breadth First Traversal](https://en.wikipedia.org/wiki/Breadth-first_search) | | | [\[X\]](breadth_first_traversal/BreadthFirstTraversal.java) | | | |
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.
@jsroyal Please take a look at space inconsistency in | | |
.
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.
Please indent using tabs. Related: Tabs vs Spaces - StackExchange
Hi! @aviaryan I think now indentation is consistant. I'm using IntelliJ IDE where the standard configuration of Java Coding Convention. |
addEdge(2, 3); | ||
addEdge(3, 3); | ||
System.out.println("Breadth First Search starting from source to destination"); | ||
ArrayList arrayList = breadthFirstSearch(1,3); |
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.
It would be great if you can make this 1, 3
.
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.
#190 (comment) is still not solved.
You may refer to this implementation if you need something.
@jsroyal: You have been inactive for 3 days now. Just as a reminder, this PR will be closed if you stay inactive for 7 days. |
README.md
Outdated
@@ -14,6 +14,7 @@ Community (college) maintained list of Algorithms and Data Structures implementa | |||
|:--------------|:----------------:|:----------------:|:----------------:|:-----------------:|:-----------------:| | |||
| [Bin Sort](http://www.cdn.geeksforgeeks.org/bucket-sort-2/)| | |[\[X\]](bin_sort/BinSort.java)| | | | |||
| [Binary Search](https://en.wikipedia.org/wiki/Binary_search_algorithm) | [\[X\]](binary_search/binary_search.c) | | [\[X\]](binary_search/BinarySearch.java) | [\[X\]](binary_search/binary_search.py) | | | |||
| [Breadth First Traversal](https://en.wikipedia.org/wiki/Breadth-first_search) | | | [\[X\]](breadth_first_traversal/BreadthFirstTraversal.java) | | | |
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.
Change this to Breadth First Search.
@jsroyal Please fix the build.
|
@aviaryan trim_trailing_whitespace I do not get it. Please mention . |
public class BreadthFirstSearch<T> { | ||
// HasMap of lists for Adjacency List Representation | ||
public HashMap<T, ArrayList<T>> adj = new HashMap<T, ArrayList<T>> (); | ||
|
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.
@jsroyal Sir, It is about removing the unnecessary whitespaces from blank lines 10 and 25.
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.
@jsroyal You have to remove whitespace after line 10 and 25 here.
the line has trailing whitespace
this line does not
this line has trailing whitespace
⬆️ Select the contents of block above to understand.
public HashMap<T, ArrayList<T>> adj = new HashMap<T, ArrayList<T>> (); | ||
//Function to add an edge | ||
public void addEdges (T source, T destination) { | ||
if (adj.containsKey (source)) { |
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.
Please remove space between function name and parameters.
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.
This issue is at a lot of places.
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.
It is too much 🔢 70
README.md
Outdated
| [Binary Search](https://en.wikipedia.org/wiki/Binary_search_algorithm) | [:white_check_mark:](binary_search/binary_search.c) | | [:white_check_mark:](binary_search/BinarySearch.java) | [:white_check_mark:](binary_search/binary_search.py) | [:white_check_mark:](binary_search/binary_search.go) | [:white_check_mark:](binary_search/binarySearch.js) | | ||
| [Coin Change Problem](http://www.algorithmist.com/index.php/Coin_Change) | [:white_check_mark:](coin_change_problem/coin_change_problem.c) | | [:white_check_mark:](coin_change_problem/CoinChange.java) | [:white_check_mark:](coin_change_problem/coin_change_problem.py) | | [:white_check_mark:](coin_change_problem/coinChangeProblem.js) | | ||
| [Counting Sort](http://www.geeksforgeeks.org/counting-sort/)| [:white_check_mark:](counting_sort/counting_sort.c) | | [:white_check_mark:](counting_sort/CountingSort.java) | | [:white_check_mark:](counting_sort/counting_sort.go) | | | ||
| [Binary Search](https://en.wikipedia.org/wiki/Binary_search_algorithm) | [:white_check_mark:](binary_search/binary_search.c) | | [:white_check_mark:](binary_search/BinarySearch.java) | [:white_check_mark:](binary_search/binary_search.py) | [:white_check_mark:](binary_search/binary_search.go) | | |
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.
The code looks good now. Please fix this diff.
for (int i = 0; i < temp.size(); i++) { | ||
if (! visited.contains(temp.get(i))) { | ||
bfsPath.add(temp.get(i)); | ||
if (temp.get(i) == destination) { |
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.
Please use .equals()
for comparison here as ==
compares object references rather than what is needed to be compared.
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.
🥇
@jsroyal sir update README its showing some unnecessary diffs. |
@jsroyal Thanks for this PR. It might have taken a long time but this was some quality code. Looking forward to more contributions from you. |
bfsPath.add(source); | ||
// mark as visited | ||
visited.add(source); | ||
int flag = 0; |
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.
If we only have to deal with 0
and 1
, we can use boolean here
Fixes #73
By submitting this pull request I confirm I've read and complied with the below declarations.
Added {Algorithm/DS name} [{Language}]
, notUpdate README.md
orAdded new code
.