better naming and comments

Signed-off-by: Shlomi Noach <[email protected]>

go/vt/graph/graph.go

@@ -104,10 +104,11 @@ func (gr *Graph[C]) HasCycles() bool {
 	return false
 }
 
-// HasCycles checks whether the given graph has a cycle or not.
+// GetCycles returns all known cycles in the graph.
+// It returns a map of vertices to the cycle they are part of.
 // We are using a well-known DFS based colouring algorithm to check for cycles.
 // Look at https://cp-algorithms.com/graph/finding-cycle.html for more details on the algorithm.
-func (gr *Graph[C]) GetCycleVertices() (vertices map[C][]C) {
+func (gr *Graph[C]) GetCycles() (vertices map[C][]C) {
 	// If the graph is empty, then we don't need to check anything.
 	if gr.Empty() {
 		return nil
@@ -121,6 +122,8 @@ func (gr *Graph[C]) GetCycleVertices() (vertices map[C][]C) {
 	for _, vertex := range gr.orderedEdged {
 		// If any vertex is still white, we initiate a new DFS.
 		if color[vertex] == white {
+			// We clone the colors because we wnt full coverage for all vertices.
+			// Otherwise, the algorithm is optimal and stop more-or-less after the first cycle.
 			color := maps.Clone(color)
 			if hasCycle, cycle := gr.hasCyclesDfs(color, vertex); hasCycle {
 				vertices[vertex] = cycle

go/vt/graph/graph_test.go

@@ -90,12 +90,12 @@ func TestIntegerGraph(t *testing.T) {
 // TestStringGraph tests that a graph with strings can be created and all graph functions work as intended.
 func TestStringGraph(t *testing.T) {
 	testcases := []struct {
-		name              string
-		edges             [][2]string
-		wantedGraph       string
-		wantEmpty         bool
-		wantHasCycles     bool
-		wantCycleVertices map[string][]string
+		name          string
+		edges         [][2]string
+		wantedGraph   string
+		wantEmpty     bool
+		wantHasCycles bool
+		wantCycles    map[string][]string
 	}{
 		{
 			name:          "empty graph",
@@ -138,7 +138,7 @@ E - F
 F - A`,
 			wantEmpty:     false,
 			wantHasCycles: true,
-			wantCycleVertices: map[string][]string{
+			wantCycles: map[string][]string{
 				"A": {"A", "B", "E", "F", "A"},
 				"B": {"B", "E", "F", "A", "B"},
 				"D": {"D", "E", "F", "A", "B", "E"},
@@ -156,14 +156,14 @@ F - A`,
 			require.Equal(t, tt.wantedGraph, graph.PrintGraph())
 			require.Equal(t, tt.wantEmpty, graph.Empty())
 			require.Equal(t, tt.wantHasCycles, graph.HasCycles())
-			if tt.wantCycleVertices == nil {
-				tt.wantCycleVertices = map[string][]string{}
+			if tt.wantCycles == nil {
+				tt.wantCycles = map[string][]string{}
 			}
-			actualCycleVertices := graph.GetCycleVertices()
-			if actualCycleVertices == nil {
-				actualCycleVertices = map[string][]string{}
+			actualCycles := graph.GetCycles()
+			if actualCycles == nil {
+				actualCycles = map[string][]string{}
 			}
-			require.Equal(t, tt.wantCycleVertices, actualCycleVertices)
+			require.Equal(t, tt.wantCycles, actualCycles)
 		})
 	}
 }