-
Notifications
You must be signed in to change notification settings - Fork 20k
Add WelshPowell
(Graph Colouring)
#5034
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
Conversation
i try to format based on clang format but i cant. Dowloaded llvm added to enviroment variables and path and then my computer finds the clang format. After that i dont know what to do. |
You can have a look at the log and introduce the required changes manually. If you are having trouble with running |
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 is just the first iteration.
src/main/java/com/thealgorithms/datastructures/graphs/Welsh_Powell.java
Outdated
Show resolved
Hide resolved
src/main/java/com/thealgorithms/datastructures/graphs/Welsh_Powell.java
Outdated
Show resolved
Hide resolved
src/test/java/com/thealgorithms/datastructures/graphs/WelshPowellAlgorithmTest.java
Outdated
Show resolved
Hide resolved
i think i changed everything you asked, please inform me if i need to change or fix anything else. |
src/main/java/com/thealgorithms/datastructures/graphs/WelshPowell.java
Outdated
Show resolved
Hide resolved
src/main/java/com/thealgorithms/datastructures/graphs/WelshPowell.java
Outdated
Show resolved
Hide resolved
src/main/java/com/thealgorithms/datastructures/graphs/WelshPowell.java
Outdated
Show resolved
Hide resolved
…ell.java Co-authored-by: Piotr Idzik <[email protected]>
…thods in order for the test class to work
…thods in order for the test class to work
src/main/java/com/thealgorithms/datastructures/graphs/WelshPowell.java
Outdated
Show resolved
Hide resolved
src/main/java/com/thealgorithms/datastructures/graphs/WelshPowell.java
Outdated
Show resolved
Hide resolved
src/main/java/com/thealgorithms/datastructures/graphs/WelshPowell.java
Outdated
Show resolved
Hide resolved
src/main/java/com/thealgorithms/datastructures/graphs/WelshPowell.java
Outdated
Show resolved
Hide resolved
src/main/java/com/thealgorithms/datastructures/graphs/WelshPowell.java
Outdated
Show resolved
Hide resolved
src/test/java/com/thealgorithms/datastructures/graphs/WelshPowellAlgorithmTest.java
Outdated
Show resolved
Hide resolved
src/main/java/com/thealgorithms/datastructures/graphs/WelshPowell.java
Outdated
Show resolved
Hide resolved
src/main/java/com/thealgorithms/datastructures/graphs/WelshPowell.java
Outdated
Show resolved
Hide resolved
src/main/java/com/thealgorithms/datastructures/graphs/WelshPowell.java
Outdated
Show resolved
Hide resolved
…ut works. (working on it :) )
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.
Regarding the general design of this class, I would suggest the following (pseudo code):
class WelshPowell {
private static class WPGraph {
// almost same as now
}
public static WPGraph makeGraph(int numberOfVertices, listOfEdges) {
// here you create the graph
}
public static int[] welshPowellColoring(WPGraph graph) {
// as it is now
}
}
Then, you could write the first test like (still pseudo code)
final var graph = WelshPowell.makeGraph(3, {{0, 1}, {1, 2}, {2, 0}});
final var colors = WelshPowell.welshPowellColoring(graph);
// check if the colors is as expected
assert(...);
Thanks to this, you will be able to get rid of some methods.
src/main/java/com/thealgorithms/datastructures/graphs/WelshPowell.java
Outdated
Show resolved
Hide resolved
src/main/java/com/thealgorithms/datastructures/graphs/WelshPowell.java
Outdated
Show resolved
Hide resolved
src/main/java/com/thealgorithms/datastructures/graphs/WelshPowell.java
Outdated
Show resolved
Hide resolved
src/main/java/com/thealgorithms/datastructures/graphs/WelshPowell.java
Outdated
Show resolved
Hide resolved
src/main/java/com/thealgorithms/datastructures/graphs/WelshPowell.java
Outdated
Show resolved
Hide resolved
src/main/java/com/thealgorithms/datastructures/graphs/WelshPowell.java
Outdated
Show resolved
Hide resolved
src/test/java/com/thealgorithms/datastructures/graphs/WelshPowellAlgorithmTest.java
Outdated
Show resolved
Hide resolved
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.
We are almost there! Let's sped some time to make everything perfect.
src/main/java/com/thealgorithms/datastructures/graphs/WelshPowell.java
Outdated
Show resolved
Hide resolved
src/main/java/com/thealgorithms/datastructures/graphs/WelshPowell.java
Outdated
Show resolved
Hide resolved
src/main/java/com/thealgorithms/datastructures/graphs/WelshPowell.java
Outdated
Show resolved
Hide resolved
src/main/java/com/thealgorithms/datastructures/graphs/WelshPowell.java
Outdated
Show resolved
Hide resolved
src/main/java/com/thealgorithms/datastructures/graphs/WelshPowell.java
Outdated
Show resolved
Hide resolved
src/main/java/com/thealgorithms/datastructures/graphs/WelshPowell.java
Outdated
Show resolved
Hide resolved
src/main/java/com/thealgorithms/datastructures/graphs/WelshPowell.java
Outdated
Show resolved
Hide resolved
src/main/java/com/thealgorithms/datastructures/graphs/WelshPowell.java
Outdated
Show resolved
Hide resolved
src/main/java/com/thealgorithms/datastructures/graphs/WelshPowell.java
Outdated
Show resolved
Hide resolved
src/test/java/com/thealgorithms/datastructures/graphs/WelshPowellAlgorithmTest.java
Outdated
Show resolved
Hide resolved
…makeGraph so the listofEdges haw exactly 2 elements, and add a more complex graph in the test class
i think i resolved almost all of the changes requested. Have a look and let me know if anything else needs to be changed. |
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 add the missing tests. By the way, in same cases you can use Commit suggestion
button. It is quite handy.
src/test/java/com/thealgorithms/datastructures/graphs/WelshPowellAlgorithmTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/thealgorithms/datastructures/graphs/WelshPowellAlgorithmTest.java
Outdated
Show resolved
Hide resolved
src/main/java/com/thealgorithms/datastructures/graphs/WelshPowell.java
Outdated
Show resolved
Hide resolved
src/test/java/com/thealgorithms/datastructures/graphs/WelshPowellAlgorithmTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/thealgorithms/datastructures/graphs/WelshPowellAlgorithmTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/thealgorithms/datastructures/graphs/WelshPowellAlgorithmTest.java
Outdated
Show resolved
Hide resolved
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.
Almost done!
src/test/java/com/thealgorithms/datastructures/graphs/WelshPowellTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/thealgorithms/datastructures/graphs/WelshPowellTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/thealgorithms/datastructures/graphs/WelshPowellTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/thealgorithms/datastructures/graphs/WelshPowellTest.java
Show resolved
Hide resolved
src/test/java/com/thealgorithms/datastructures/graphs/WelshPowellTest.java
Show resolved
Hide resolved
src/main/java/com/thealgorithms/datastructures/graphs/WelshPowell.java
Outdated
Show resolved
Hide resolved
src/main/java/com/thealgorithms/datastructures/graphs/WelshPowell.java
Outdated
Show resolved
Hide resolved
src/test/java/com/thealgorithms/datastructures/graphs/WelshPowellTest.java
Show resolved
Hide resolved
i added the complex test based on the website, add the blank_color method, updated the comments on the precolored test, and followed all your suggestions. Have a look for any minor changes needed but all in all I think it is ready. |
WelshPowell
(Graph Colouring)
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.
Good work!
For a future project you can consider adding a method trying to determine if the returned coloring is optimal.
clang-format -i --style=file path/to/your/file.java