algorithms icon indicating copy to clipboard operation
algorithms copied to clipboard

Enforce the consistency between function parameters and algorithm input

Open unfode opened this issue 3 years ago • 4 comments
trafficstars

Take Breadth-First Search/tree.js (shown below) as an example.

The input to the BFS algorithm is a graph (or tree) and a starting node, while the BFS function only takes the starting node s as parameter, leaving the graph G as a global variable. In my humble opinion, it would be better to avoid such mismatch, which can potentially confuse learners.

Note: This mismatch also appears in some other algorithm code.

https://github.com/algorithm-visualizer/algorithms/blob/fe3adcf890da652e5cda842d7f90b50fa7242e3a/Brute%20Force/Breadth-First%20Search/tree.js#L1-L50

unfode avatar Apr 13 '22 06:04 unfode

@64json @MarkKoz @nem035 @duaraghav8 @TornjV

unfode avatar Apr 17 '22 01:04 unfode

Makes sense and feel free to contribute yourself.

FYI none of us are actively maintaining the project. You don't need to tag us on every issue, we will take a look when we get time.

64json avatar Apr 17 '22 06:04 64json

Got it. I won't tag you in future issues.

And I will rewrite some algorithm code and make pull requests :-)

unfode avatar Apr 17 '22 07:04 unfode

Here's a related issue: where should we place code for tracer initialization and tracer.set()?

To me it seems more natural to place tracer code inside the algorithm function. For Breadth-First Search/tree.js, it would be

 // import visualization libraries { 
 const { Tracer, GraphTracer, LogTracer, Layout, VerticalLayout } = require('algorithm-visualizer'); 
 // } 
  
 const G = [ // G[i][j] indicates whether the path from the i-th node to the j-th node exists or not 
   [0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0], 
   [0, 0, 0, 1, 1, 0, 0, 0, 0, 0, 0], 
   [0, 0, 0, 0, 0, 1, 1, 0, 0, 0, 0], 
   [0, 0, 0, 0, 0, 0, 0, 1, 1, 0, 0], 
   [0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1], 
   [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0], 
   [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0], 
   [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0], 
   [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0], 
   [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0], 
   [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0], 
 ]; 

 function BFS(G, s) { // s = start node 

   // define tracer variables { 
   const tracer = new GraphTracer(); 
   const logger = new LogTracer(); 
   tracer.log(logger); 
   Layout.setRoot(new VerticalLayout([tracer, logger])); 
   tracer.set(G); 
   tracer.layoutTree(0); 
   Tracer.delay(); 
   // } 

   const Q = []; 
   Q.push(s); // add start node to queue 
   // visualize { 
   tracer.visit(s); 
   Tracer.delay(); 
   // } 
   while (Q.length > 0) { 
     const node = Q.shift(); // dequeue 
     for (let i = 0; i < G[node].length; i++) { 
       if (G[node][i]) { // if current node has the i-th node as a child 
         Q.push(i); // add child node to queue 
         // visualize { 
         tracer.visit(i, node); 
         Tracer.delay(); 
         // } 
       } 
     } 
   } 
 } 
  
 BFS(G, 0); 

The reason is that we may want to trace, say, an array declared inside the algorithm function.

A case in point is the array S in Dijkstra's Shortest Path/code.js (shown below). If we enforce the correspondence between algorithm and function, S should be declared inside the algorithm function, which requires (at least) tracerS.set(S) be placed inside the function.

https://github.com/algorithm-visualizer/algorithms/blob/fe3adcf890da652e5cda842d7f90b50fa7242e3a/Greedy/Dijkstra's%20Shortest%20Path/code.js#L1-L88

unfode avatar Apr 17 '22 07:04 unfode