Molinillo icon indicating copy to clipboard operation
Molinillo copied to clipboard

Unclear assignment of root in dependency_graph.add_child_vertex

Open baron1405 opened this issue 2 years ago • 0 comments

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.

baron1405 avatar Oct 17 '23 01:10 baron1405