Added function and tests for cyclic dependency check #25
Conversation
dependency-graph.js
Outdated
| const VISITED = 2; | ||
| const statusMap = new Map(Object.keys(graph).map(key => [key, UNVISITED])); | ||
|
|
||
| // Utility function for depth first search |
There was a problem hiding this comment.
convert these to jsdocs?
/**
* Utility function for depth first search. Updates `cycle` to keep track of potential circular dependency.
* @param {unknown} node The node to visit
* @param {Array<unknown>} neighbours The neighbours of the node being visited.
* @returns {undefined} Nothing is returned
* @throws {Error} If a cycle is encountered while doing DFS
*/
function dfs(node, neighbours){
test/dependency-graph.js
Outdated
| const graph = depGraph.createDependencyTree(expression); | ||
| const cycle = depGraph.checkForCyclicDependency(graph); | ||
| expect(cycle).to.deep.equal([ | ||
| 'a', |
There was a problem hiding this comment.
this is a bit weird...
it seems that a has a circular dependency but its actually x that has it?
shouldn't the execution just stop when the first x is encountered?
There was a problem hiding this comment.
maybe clarify this a bit in the comment describing the checkForCyclicDependency function?
There was a problem hiding this comment.
I'm returning the entire dfs path now. I think it'll make more sense to return the cyclic part of it only.
Also, dependency graph is of the format baseVar -> [ vars affected by baseVar ], thats why the cycle is sort of reverse of what you'd expect.
Reversing cycle array just before returning should do the trick.
There was a problem hiding this comment.
Just to confirm, here the expected cycle is [ x, z, p, q, x ] right ? @codebreach
Add function
checkForCyclicDependencywhich checks if the given graph has cycle in it using Depth First Search. If cycle is found, it return the cyclic path, otherwise returns[].