-
Notifications
You must be signed in to change notification settings - Fork 19
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 a function to delete a Node and its associated Edges #24
Conversation
// Remove all the edges to the Node | ||
for parent, edgeList := range g.edgesFrom { | ||
for i, edge := range edgeList { | ||
if edge.to.id == id { |
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.
looks like this can be done more efficiently; break the loop after deletion
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.
I'm not sure about this one, because two nodes A & B can have several edges going from A to B (happens in my project). The complexity is bad (the average is the number of edges...) but I don't think there is another way to do so, and removing a node should be a one-time operation, not something to be optimized. But that's your lib, don't hesitate to tell me what you think about this :)
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 probably right. and again, the whole lib is not build for performance.
Coverage is now 92.2, sorry for the force push, i just forgot things in my last commit so I --amend'ed them |
thank you for contributing @EwenQuim |
Added a function and its corresponding Test.
Tests pass 100%.
I need it for my project, is it ok for you to integrate it in your lib ?
A fellow golang dev,
Best