Content-Length: 430543 | pFad | https://github.com/TheAlgorithms/Go/pull/374

34 feat: kruskal's algorithm using DSU by viveknathani · Pull Request #374 · TheAlgorithms/Go · GitHub
Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: kruskal's algorithm using DSU #374

Merged
merged 13 commits into from
Oct 7, 2021
Merged

feat: kruskal's algorithm using DSU #374

merged 13 commits into from
Oct 7, 2021

Conversation

viveknathani
Copy link
Contributor

Hi guys! I saw that Kruskal's algorithm was missing from the list. So I wrote an implementation using the disjoint set union data structure. Also, it would be amazing if you could add the hacktoberfest-accepted label to it.

References:

  1. https://cp-algorithms.com/data_structures/disjoint_set_union.html
  2. https://cp-algorithms.com/graph/mst_kruskal_with_dsu.html

@lgtm-com
Copy link

lgtm-com bot commented Oct 6, 2021

This pull request introduces 2 alerts when merging eb3a8ac into e14ac15 - view on LGTM.com

new alerts:

  • 2 for Useless assignment to local variable

raklaptudirm
raklaptudirm requested changes Oct 6, 2021
View reviewed changes
Copy link
Member

@raklaptudirm raklaptudirm 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 some links for reference.

@raklaptudirm raklaptudirm added Changes Required enhancement Reviewed This pull request has been reviewed. labels Oct 6, 2021
@viveknathani
Copy link
Contributor Author

Please add some links for reference.

done

raklaptudirm
raklaptudirm previously approved these changes Oct 6, 2021
View reviewed changes
tjgurwara99
tjgurwara99 requested changes Oct 6, 2021
View reviewed changes
Copy link
Member

@tjgurwara99 tjgurwara99 left a comment

Choose a reason for hiding this comment

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

Good implementation. I recommend trying to use Go's powerful syntax capabilities instead of just refactoring c++ style code to go. Think of algorithms that are as easy to understand as possible when reading.

@viveknathani
Copy link
Contributor Author

@tjgurwara99 I think all the requested changes are complete. Please take a look. Thanks for the tips you gave along the way!

tjgurwara99
tjgurwara99 requested changes Oct 7, 2021
View reviewed changes
Copy link
Member

@tjgurwara99 tjgurwara99 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Other than those everything else looks good. The reason I'm asking for these changes is because I'm trying to get this repository to work nicely with go playground. For example, take a look at this one. This is using the alpha version of this repo released(?) a few months back and can easily be used to work with it to try and test out this repo functions and implementations without cloning it locally.

tjgurwara99
tjgurwara99 approved these changes Oct 7, 2021
View reviewed changes
Copy link
Member

@tjgurwara99 tjgurwara99 left a comment

Choose a reason for hiding this comment

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

LGTM

Maybe an improvement could be made by making use of dynamic size of slices but... that is for another PR. This one can be merged now.

@tjgurwara99
Copy link
Member

tjgurwara99 commented Oct 7, 2021 •
edited
Loading

@viveknathani you might need to do a git pull on your local - I mistakenly synchronised your branch with our master branch.

@siriak could you also take a look at this PR to see if it needs any changes, if not it can be merged 😄

siriak
siriak approved these changes Oct 7, 2021
View reviewed changes
Copy link
Member

@siriak siriak left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@siriak siriak merged commit af4ca77 into TheAlgorithms:master Oct 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tjgurwara99 tjgurwara99 tjgurwara99 approved these changes

@siriak siriak siriak approved these changes

@raklaptudirm raklaptudirm raklaptudirm left review comments

Assignees
No one assigned
Labels
enhancement Reviewed This pull request has been reviewed.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@viveknathani @tjgurwara99 @siriak @raklaptudirm








ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: https://github.com/TheAlgorithms/Go/pull/374

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy