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.

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

Unclear assignment of root in dependency_graph.add_child_vertex #172

Open
baron1405 opened this issue Oct 17, 2023 · 0 comments
Open

Unclear assignment of root in dependency_graph.add_child_vertex #172

baron1405 opened this issue Oct 17, 2023 · 0 comments

Comments

@baron1405
Copy link

In the dependency_graph.add_child_vertex the variable root is assigned:

root = !parent_names.delete(nil) { true }

This assignment does not makes sense to me.

Test Case root
![].delete(nil) { true } false
![nil].delete(nil) { true } true
!["a", "b"].delete(nil) { true } false
!["a", "b", nil].delete(nil) { true } true
![nil, "a", "b"].delete(nil) { true } true

If the intent is to allow a root vertex to be created using the add_child_vertex method, the criteria is that the parent_names list is empty (or for some reason only contains nil values). In general, it seems like creating a root vertex in this method is undesirable and an exception should be thrown if the parent names list is empty. In any case, the current implementation is either incorrect or a comment should be added to clarify the intent.

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

No branches or pull requests

1 participant