Skip to content
This repository has been archived by the owner on Apr 26, 2019. It is now read-only.

search: protect existing nodes during graph construction #36

Merged
merged 1 commit into from
May 15, 2015
Merged

search: protect existing nodes during graph construction #36

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion search/search_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,9 @@ func TestTarjanSCC(t *testing.T) {
for i, test := range tarjanTests {
g := concrete.NewDirectedGraph()
for u, e := range test.g {
g.AddNode(concrete.Node(u))
if !g.NodeExists(concrete.Node(u)) {
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't this imply that the test is wrong, and thus a panic is more applicable?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, it may be the case that an edge has been created to a node that has not yet been seen due to the test.g range loop. For example in the first Tarjan test case:

g: []set{
    0: linksTo(1),
    1: linksTo(2, 7),
    2: linksTo(3, 6),
    3: linksTo(4),
    4: linksTo(2, 5),
    6: linksTo(3, 5),
    7: linksTo(0, 6),
},

The first edge {0,1} creates the node 1, so by the time we get to adding node 1 explicitly, it is already there.

The API allows creation of both nodes implicitly during an AddDirectedEdge call and I will probably change these and others to that later, but here I want to be explicit until the API redesign has been more fully fleshed out (see #45 in particular).

Copy link
Member Author

Choose a reason for hiding this comment

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

@btracey Are you OK with this to be merged?

g.AddNode(concrete.Node(u))
}
for v := range e {
if !g.NodeExists(concrete.Node(v)) {
g.AddNode(concrete.Node(v))
Expand Down