-
Notifications
You must be signed in to change notification settings - Fork 26
Refactor: Reduce Cognitive Complexity in BuildGraph Method #701
Copy link
Copy link
Labels
C#C# related codeC# related codeMaintainabilitybugSomething isn't workingSomething isn't workinggood first issueGood for newcomersGood for newcomershelp wantedExtra attention is neededExtra attention is neededrefactoringRefactoring codeRefactoring code
Description
What version of FlowSynx?
1.2.3
Describe the bug
The BuildGraph method currently has a Cognitive Complexity of 17, which exceeds the allowed threshold of 15. This makes the method harder to understand, maintain, and test.
We should refactor the method to reduce nesting, simplify loops, and possibly extract helper functions for clarity and maintainability.
File: src/FlowSynx.Infrastructure/Workflow/WorkflowValidator.cs
Method: BuildGraph
Current Implementation
private Dictionary<string, List<string>> BuildGraph(IEnumerable<WorkflowTask> tasks, out Dictionary<string, int> inDegree)
{
var graph = new Dictionary<string, List<string>>();
inDegree = new Dictionary<string, int>();
foreach (var task in tasks)
{
if (!graph.ContainsKey(task.Name))
graph[task.Name] = new List<string>();
inDegree.TryAdd(task.Name, 0);
// Add standard dependencies (edges: dep -> task)
foreach (var dep in task.Dependencies)
{
if (!graph.ContainsKey(dep))
graph[dep] = new List<string>();
graph[dep].Add(task.Name);
inDegree[task.Name] = inDegree.GetValueOrDefault(task.Name) + 1;
}
// Add conditional branches (edges: task -> target)
if (task.ConditionalBranches is { Count: > 0 })
{
foreach (var target in task.ConditionalBranches.Select(branch => branch.TargetTaskName))
{
if (!graph.ContainsKey(target))
graph[target] = new List<string>();
graph[task.Name].Add(target);
inDegree.TryAdd(target, 0);
inDegree[target]++;
}
}
}
return graph;
}Proposed Fix
private Dictionary<string, List<string>> BuildGraph(IEnumerable<WorkflowTask> tasks, out Dictionary<string, int> inDegree)
{
var graph = new Dictionary<string, List<string>>();
inDegree = new Dictionary<string, int>();
foreach (var task in tasks)
{
EnsureNodeExists(graph, inDegree, task.Name);
AddDependencies(task, graph, inDegree);
AddConditionalBranches(task, graph, inDegree);
}
return graph;
}
private static void EnsureNodeExists(
Dictionary<string, List<string>> graph,
Dictionary<string, int> inDegree,
string node)
{
if (!graph.ContainsKey(node))
graph[node] = new List<string>();
inDegree.TryAdd(node, 0);
}
private static void AddDependencies(
WorkflowTask task,
Dictionary<string, List<string>> graph,
Dictionary<string, int> inDegree)
{
foreach (var dep in task.Dependencies)
{
EnsureNodeExists(graph, inDegree, dep);
graph[dep].Add(task.Name);
inDegree[task.Name] = inDegree.GetValueOrDefault(task.Name) + 1;
}
}
private static void AddConditionalBranches(
WorkflowTask task,
Dictionary<string, List<string>> graph,
Dictionary<string, int> inDegree)
{
if (task.ConditionalBranches is not { Count: > 0 })
return;
foreach (var target in task.ConditionalBranches.Select(branch => branch.TargetTaskName))
{
EnsureNodeExists(graph, inDegree, target);
graph[task.Name].Add(target);
inDegree[target]++;
}
}Acceptance Criteria
- Cognitive Complexity ≤ 15
- All existing unit tests pass
- Readability and maintainability improved
- No change in behavior or logic
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
C#C# related codeC# related codeMaintainabilitybugSomething isn't workingSomething isn't workinggood first issueGood for newcomersGood for newcomershelp wantedExtra attention is neededExtra attention is neededrefactoringRefactoring codeRefactoring code