Skip to content

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

Merged
merged 26 commits into from
Feb 12, 2024
Merged

Add WelshPowell (Graph Colouring) #5034

merged 26 commits into from
Feb 12, 2024

Conversation

straf10
Copy link
Contributor

@straf10 straf10 commented Jan 31, 2024

  • I have read CONTRIBUTING.md.
  • This pull request is all my own work -- I have not plagiarized it.
  • All filenames are in PascalCase.
  • All functions and variable names follow Java naming conventions.
  • All new algorithms have a URL in their comments that points to Wikipedia or other similar explanations.
  • All new code is formatted with clang-format -i --style=file path/to/your/file.java

@straf10
Copy link
Contributor Author

straf10 commented Jan 31, 2024

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.

@vil02
Copy link
Member

vil02 commented Jan 31, 2024

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 clang-format, have a look here: #4438 (comment).

Copy link
Member

@vil02 vil02 left a 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.

@straf10 straf10 requested a review from vil02 January 31, 2024 23:06
@straf10
Copy link
Contributor Author

straf10 commented Jan 31, 2024

i think i changed everything you asked, please inform me if i need to change or fix anything else.
Thanks in advance.

@straf10 straf10 changed the title Welsh Power ALgorithm / Graph Colouring Welsh Power Algorithm / Graph Colouring Jan 31, 2024
@straf10 straf10 changed the title Welsh Power Algorithm / Graph Colouring Welsh Powell Algorithm / Graph Colouring Jan 31, 2024
Copy link
Member

@vil02 vil02 left a 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.

Copy link
Member

@vil02 vil02 left a 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.

…makeGraph so the listofEdges haw exactly 2 elements, and add a more complex graph in the test class
@straf10
Copy link
Contributor Author

straf10 commented Feb 6, 2024

i think i resolved almost all of the changes requested. Have a look and let me know if anything else needs to be changed.

Copy link
Member

@vil02 vil02 left a 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.

Copy link
Member

@vil02 vil02 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost done!

@straf10
Copy link
Contributor Author

straf10 commented Feb 12, 2024

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.

@vil02 vil02 changed the title Welsh Powell Algorithm / Graph Colouring Add WelshPowell (Graph Colouring) Feb 12, 2024
Copy link
Member

@vil02 vil02 left a 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.

@vil02 vil02 merged commit 47a9b1b into TheAlgorithms:master Feb 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants