How should the ViewModel close the form?

I was inspired by Thejuan's answer to write a simpler attached property. No styles, no triggers; instead, you can just do this:

<Window ...        xmlns:xc="clr-namespace:ExCastle.Wpf"        xc:DialogCloser.DialogResult="{Binding DialogResult}">

This is almost as clean as if the WPF team had gotten it right and made DialogResult a dependency property in the first place. Just put a bool? DialogResult property on your ViewModel and implement INotifyPropertyChanged, and voilà, your ViewModel can close the Window (and set its DialogResult) just by setting a property. MVVM as it should be.

Here's the code for DialogCloser:

using System.Windows;namespace ExCastle.Wpf{    public static class DialogCloser    {        public static readonly DependencyProperty DialogResultProperty =            DependencyProperty.RegisterAttached(                "DialogResult",                typeof(bool?),                typeof(DialogCloser),                new PropertyMetadata(DialogResultChanged));        private static void DialogResultChanged(            DependencyObject d,            DependencyPropertyChangedEventArgs e)        {            var window = d as Window;            if (window != null)                window.DialogResult = e.NewValue as bool?;        }        public static void SetDialogResult(Window target, bool? value)        {            target.SetValue(DialogResultProperty, value);        }    }}

I've also posted this on my blog.

From my perspective the question is pretty good as the same approach would be used not only for the "Login" window, but for any kind of window. I've reviewed a lot of suggestions and none are OK for me. Please review my suggestion that was taken from the MVVM design pattern article.

Each ViewModel class should inherit from WorkspaceViewModel that has the RequestClose event and CloseCommand property of the ICommand type. The default implementation of the CloseCommand property will raise the RequestClose event.

In order to get the window closed, the OnLoaded method of your window should be overridden:

void CustomerWindow_Loaded(object sender, RoutedEventArgs e){    CustomerViewModel customer = CustomerViewModel.GetYourCustomer();    DataContext = customer;    customer.RequestClose += () => { Close(); };}

or OnStartup method of you app:

    protected override void OnStartup(StartupEventArgs e)    {        base.OnStartup(e);        MainWindow window = new MainWindow();        var viewModel = new MainWindowViewModel();        viewModel.RequestClose += window.Close;        window.DataContext = viewModel;        window.Show();    }

I guess that RequestClose event and CloseCommand property implementation in the WorkspaceViewModel are pretty clear, but I will show them to be consistent:

public abstract class WorkspaceViewModel : ViewModelBase// There's nothing interesting in ViewModelBase as it only implements the INotifyPropertyChanged interface{    RelayCommand _closeCommand;    public ICommand CloseCommand    {        get        {            if (_closeCommand == null)            {                _closeCommand = new RelayCommand(                   param => Close(),                   param => CanClose()                   );            }            return _closeCommand;        }    }    public event Action RequestClose;    public virtual void Close()    {        if ( RequestClose != null )        {            RequestClose();        }    }    public virtual bool CanClose()    {        return true;    }}

And the source code of the RelayCommand:

public class RelayCommand : ICommand{    #region Constructors    public RelayCommand(Action<object> execute, Predicate<object> canExecute)    {        if (execute == null)            throw new ArgumentNullException("execute");        _execute = execute;        _canExecute = canExecute;    }    #endregion // Constructors    #region ICommand Members    [DebuggerStepThrough]    public bool CanExecute(object parameter)    {        return _canExecute == null ? true : _canExecute(parameter);    }    public event EventHandler CanExecuteChanged    {        add { CommandManager.RequerySuggested += value; }        remove { CommandManager.RequerySuggested -= value; }    }    public void Execute(object parameter)    {        _execute(parameter);    }    #endregion // ICommand Members    #region Fields    readonly Action<object> _execute;    readonly Predicate<object> _canExecute;    #endregion // Fields}

P.S. Don't treat me badly for those sources! If I had them yesterday that would have saved me a few hours...

P.P.S. Any comments or suggestions are welcome.

There are a lot of comments arguing the pros and cons of MVVM here. For me, I agree with Nir; it's a matter of using the pattern appropriately and MVVM doesn't always fit. People seems to have become willing to sacrifice all of the most important principles of software design JUST to get it to fit MVVM.

That said,..i think your case could be a good fit with a bit of refactoring.

In most cases I've come across, WPF enables you to get by WITHOUT multiple Windows. Maybe you could try using Frames and Pages instead of Windows with DialogResults.

In your case my suggestion would be have LoginFormViewModel handle the LoginCommand and if the login is invalid, set a property on LoginFormViewModel to an appropriate value (false or some enum value like UserAuthenticationStates.FailedAuthentication). You'd do the same for a successful login (true or some other enum value). You'd then use a DataTrigger which responds to the various user authentication states and could use a simple Setter to change the Source property of the Frame.

Having your login Window return a DialogResult i think is where you're getting confused; that DialogResult is really a property of your ViewModel. In my, admittedly limited experience with WPF, when something doesn't feel right it usually because I'm thinking in terms of how i would've done the same thing in WinForms.

Hope that helps.