Code Analysis AvoidExcessiveComplexity - Just Setting Commands

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());
}

score 4 · Accepted Answer · answered Apr 28 '12 at 22:40

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.

Alexei Levenkov · Answer 2 · 2012-04-29T17:01:46.180

1

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()) {}

edited Apr 29 '12 at 17:01

answered Apr 28 '12 at 19:02

Alexei Levenkov

98,904
14
127
179

I do indeed have control over RelayCommand. I don't understand though, I'm already using it with this constructor: public RelayCommand(Action – Bob Horn Apr 28 '12 at 19:09
You are converting your zero-argument functions to one-argument once for Action – Alexei Levenkov Apr 28 '12 at 19:12
I think I see what you're saying now. Because my Action and Func don't use the parameters, remove them. The only issue is that System.Windows.Input.ICommand specifies this: bool CanExecute(object parameter). And RelayCommand implements ICommand for WPF binding. – Bob Horn Apr 28 '12 at 19:44
I'm trying to suggest to make whatever conversion you need to inside of new construtor instead repeating in each call to existing constructor. – Alexei Levenkov Apr 28 '12 at 19:55
Okay, so if I create the constructor to take just Action, how do I convert that to Action – Bob Horn Apr 28 '12 at 20:05
? You are already doing it in your current code... _ => action() – Alexei Levenkov Apr 29 '12 at 02:55
I'm sorry, but I'm a bit lost. The constructor of RelayCommand has to set this field: readonly Action – Bob Horn Apr 29 '12 at 13:44
Thank you! That worked. Well, the action part anyway. The second parameter of the constructor is Predicate – Bob Horn Apr 29 '12 at 22:02

Code Analysis AvoidExcessiveComplexity - Just Setting Commands

2 Answers2