Skip to content

Fix findCycles missing self-loops created with named edges#235

Open
chatman-media wants to merge 1 commit into
dagrejs:masterfrom
chatman-media:fix/find-cycles-named-self-loop
Open

Fix findCycles missing self-loops created with named edges#235
chatman-media wants to merge 1 commit into
dagrejs:masterfrom
chatman-media:fix/find-cycles-named-self-loop

Conversation

@chatman-media

Copy link
Copy Markdown

Fixes #111.

findCycles decides whether a single-node strongly-connected component is a cycle by calling graph.hasEdge(v, v). That overload only matches the default (unnamed) edge, so a self-loop added as a named edge in a multigraph (setEdge(v, v, label, name)) is missed and the node is wrongly reported as cycle-free.

Switched the check to graph.outEdges(v, v), which returns every edge between v and itself regardless of name, so any self-loop is detected. Behaviour for unnamed self-loops is unchanged.

Repro (before this PR returns []):

const g = new Graph({ multigraph: true });
g.setEdge('a', 'a', 'label', 'name');
findCycles(g); // expected [['a']]

Added a regression test (fails without the fix). Full suite (256 tests), typecheck and lint all pass.

findCycles checked for a single-node component's self-loop via
graph.hasEdge(v, v), which only matches the default (unnamed) edge.
A self-loop added as a named edge in a multigraph (setEdge(v, v, label, name))
was therefore missed, so findCycles returned no cycle for such a node.

Use graph.outEdges(v, v), which returns every edge between v and itself
regardless of edge name, so any self-loop is detected.

Fixes dagrejs#111
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

Successfully merging this pull request may close these issues.

findCycles ignores self-referencing cycles for graphs with named edges

1 participant