Question

I have a WPF form with 16 buttons on it. When my view model initializes, I need to set all 16 as RelayCommand objects. This is all my Initialize() method does, yet that causes the code analysis error CA1502:AvoidExcessiveComplexity.

Is this a good case for suppressing a CA warning, or is there a more elegant way to set these commands without causing the CA violation?

[SuppressMessage("Microsoft.Maintainability", "CA1502:AvoidExcessiveComplexity", Justification = "Simply setting the commands")]
private void Initialize()
{
    this.AddApplicationCommand      = new RelayCommand(_ => AddApplication());
    this.DeleteApplicationCommand   = new RelayCommand(_ => DeleteApplication(), _ => ApplicationIsSelected);
    this.RefreshApplicationsCommand = new RelayCommand(_ => RefreshApplications());
    this.SaveApplicationCommand     = new RelayCommand(_ => SaveApplication(), _ => ApplicationIsSelected);

    this.ForceInstallationCommand       = new RelayCommand(_ => ForceInstallation(), _ => ApplicationIsSelected);
    this.DeleteForceInstallationCommand = new RelayCommand(_ => DeleteForceInstallation(), _ => ApplicationIsSelectedAndForceExists());

    this.AddTaskCommand            = new RelayCommand(_ => AddTask(), _ => ApplicationIsSelected);
    this.EditTaskCommand           = new RelayCommand(_ => EditTask(), _ => TaskIsSelected());
    this.DeleteTaskCommand         = new RelayCommand(_ => DeleteTask(), _ => TaskIsSelected());
    this.ImportTasksCommand        = new RelayCommand(_ => ImportTasks(), _ => ApplicationIsSelected);
    this.ExportTasksCommand        = new RelayCommand(_ => ExportTasks(), _ => TaskIsSelected());
    this.ImportLegacyTasksCommand  = new RelayCommand(_ => ImportLegacyTasks(), _ => ApplicationIsSelected);            
    this.MoveTaskUpCommand         = new RelayCommand(_ => MoveRowUp(), _ => TaskIsSelected());
    this.MoveTaskDownCommand       = new RelayCommand(_ => MoveRowDown(), _ => TaskIsSelected());

    this.AddVariableGroupCommand    = new RelayCommand(_ => AddVariableGroup());
    this.RemoveVariableGroupCommand = new RelayCommand(_ => RemoveVariableGroup(), _ => VariableGroupIsSelected());
}
Was it helpful?

Solution

This is a false positive due to the use of anonymous methods. The rule does not recognize the compiler-generated branching as generated code. See https://connect.microsoft.com/VisualStudio/feedback/details/555560/method-using-many-lambda-expressions-causes-high-cyclomatic-complexity and https://connect.microsoft.com/VisualStudio/feedback/details/295703/incorrect-cyclomatic-complexity-with-lambdas for existing bug reports.

OTHER TIPS

Looks readable as it is, so suppressing would be ok to me (assuming you can't change RelayCommand).

If you can control RelayCommand class - add constructor RealayCommand(Action, Func<bool>) to eliminate extra wrapper lambda/delegates creation.

If you expect any more buttons consider switching to table with { button, action, enabled } entries.

EDIT: To simplify code by removing delegates creation on each line you can change code from

new RelayCommand(_ => MoveRowDown(), _ => TaskIsSelected());

to

new RelayCommand(MoveRowDown, TaskIsSelected);

By adding new constructor that sets the fileds itself:

public RelayCommand(Action action, Func<bool> enabled)
{
  this._execute = _ => action();
  this._enabled = _ => enabled();
}

or new constructor that calls existing constructor:

public RelayCommand(Action action, Func<bool> enabled)
 : this(_ => MoveRowDown(), _ => TaskIsSelected()) {}
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top