2

I found this: Close Window from ViewModel which gets me started down the path of modifying my DelegateCommand class to handle parameters. But I am not able to get the syntax worked out.

Here is my DelegateCommand class, and the DelegateCommand class that I'm trying to create with little success:

    public class DelegateCommand : ICommand
    {
        private readonly Action _action;

        public DelegateCommand(Action action)
        {
            _action = action;
        }

        public void Execute(object parameter)
        {
            _action();
        }

        public bool CanExecute(object parameter)
        {
            return true;
        }

#pragma warning disable 67
        public event EventHandler CanExecuteChanged { add { } remove { } }
#pragma warning restore 67
    }

    public class DelegateCommand<T> : ICommand
    {
        private readonly Action _action;

        public DelegateCommand(Action action)
        {
            _action = action;
        }

        public void Execute(object parameter)
        {
            _action();
        }

        public bool CanExecute(object parameter)
        {
            return true;
        }

#pragma warning disable 67
        public event EventHandler CanExecuteChanged { add { } remove { } }
#pragma warning restore 67
    }

And here is what I do in the viewmodel:

public ICommand RowEditEndingAction
{
    get { return new DelegateCommand(RowEditEnding); }
}

public ICommand UpdateDatabaseClick
{
    get { return new DelegateCommand<object>(UpdateDatabase); } //THIS LINE HAS THE ERROR
}

And the actual method that will get called:

public void UpdateDatabase(object parameter)
{
    Window w = (Window)parameter;
    // A bunch of stuff that works is removed for brevity
    w.Close();
}

The compiler does not like my UpdateDatabaseClick, specifically saying there is something wrong with the arguments to DelegateCommand, but I am at a loss as to what I am doing wrong (though I am thinking it is syntax . . . ). What do I need to change? This all worked before I added the parameter to UpdateDatabase, and just had the DelegateCommand (no template class). But in that case I could not close the window.

Community
  • 1
  • 1
Paul Gibson
  • 622
  • 1
  • 9
  • 23
  • *What exactly* is the compiler saying? What messages? – O. R. Mapper Mar 23 '15 at 23:10
  • 1
    When I hover over the "new DelegateCommand(UpdateDatabase)" piece of my ICommand, which has the red squiggly under, it says that "The best overloaded method for DelegateCommand.DelegateCommand(System Action) has some invalid arguments. – Paul Gibson Mar 23 '15 at 23:15
  • Every time someone accesses your properties they get a new DelegateCommand. That seems... not to be the intent. Make them read only {get;private set;} and set them in your constructor. Or leak and bug away. –  Mar 24 '15 at 13:31
  • @Will, Thanks . . . I was noticing that most examples I have seen lately do it the way you describe. When debugging I think that I noticed the ICommand properties get initialized when the window calls InitializeComponent(), and then subsequent actions to the bound control go straight to the actual function. So it is unclear to me what the distinction between the methods really would be. – Paul Gibson Mar 24 '15 at 15:11
  • Every... time... someone accesses the ICommand property... a new instance is created. In an extremely simple application, this may only happen once. There is no guarantee otherwise. If you don't understand this, or the implications of leaking memory by doing this, or having ten or twenty instances all hooked to the same button, then by all means go right ahead doing this. It's not like poor coding practices will come back to bite you hard on the ass later. –  Mar 24 '15 at 15:34
  • @Will I understand what you are saying and I wondered the same thing when I found the example that had this style. The only time an ICommand instance is created is when the Window the view model is instantiated in is initialized. After that the ICommand is not called, but the method passed in DelegateCommand is. To me this indicates that there will only be a single instance of the ICommand, and I have not yet been able to create a case where a second one gets created. I am interested to hear more detail, but on the face of it I am not sure you are correct. – Paul Gibson Mar 24 '15 at 15:48
  • I am correct, and you are free to continue using bad coding practices. I'll give you one example where this will screw you--DataTemplates with elements that bind to ICommand properties. Now, if you will excuse me, I smell bad code elsewhere, and must leave. Excelsior! And good day to you. –  Mar 24 '15 at 16:10

2 Answers2

7

Here's the constructor of your DelegateCommand<T> class that you are calling:

public DelegateCommand(Action action)

And here is how you call it:

new DelegateCommand<object>(UpdateDatabase)

In there, UpdateDatabase is declared as follows:

public void UpdateDatabase(object parameter)

Now, the constructor you are invoking expects an Action. That is a parameterless method without a return value.

However, you are passing in a method with one parameter. That is what the compiler complains about.

What you actually probably want to do is to accept any method with one parameter - for that, you can use the type Action<T>. As your parameter should presumeably have the same type that is passed as a type argument to your DelegateCommand<T> class, you can declare your constructor like this:

public DelegateCommand(Action<T> action)

Now, you also need to update the type of your backing field where you store the action:

private readonly Action<T> _action;

Lastly, as _action now expects an argument, you need to pass that argument when invoking _action in DelegateCommand<T>.Execute. Normally, you want to hand over the parameter object you receive as an argument to the Execute method. However, that value is always typed to object, whereas you want to work with a strongly-typed value of type T in your method. Therefore, you will also have to add an additional cast:

public void Execute(object parameter)
{
    _action((T)parameter);
}
O. R. Mapper
  • 20,083
  • 9
  • 69
  • 114
0

I suggest you try Prism framework. It has all the component, tools, you need to work with WPF application with MVVM model.

Thai Anh Duc
  • 524
  • 6
  • 12
  • 1
    Thanks for your response . . . I am trying to learn as much as I can, so in this case wanted to avoid the various frameworks. I have also implemented messaging between view models without the various messaging frameworks . . . very enlightening. – Paul Gibson Mar 23 '15 at 23:28