Is it right way to make Window to Singleton and call Visibility and Shutdown() methods directly in WPF?

Question

There is a parent window that is the basis of the program, and there are several sub windows under it. Editor is one of the sub window and I'm making it. The code is as follows:

// This is Code-Behind
public partial class MyEditor : Window
{
    private static readonly Lazy<MyEditor> lazy = new Lazy<MyEditor>(() => new MyEditor());
    public static MyEditor Instance { get => lazy.Value; }

    public EditorViewModel EditorViewModel { get; } = new EditorViewModel();

    private MyEditor()
    {
        InitializeComponent();

        DataContext = this;

        Application.Current.MainWindow.Closing += MainWindow_Closing;
    }

    public void Show()
    {
        Visibility = Visibility.Visible;
    }

    private void Window_Closing(object sender, CancelEventArgs e)
    {
        e.Cancel = true;
        Visibility = Visibility.Hidden;
    }

    private void MainWindow_Closing(object sender, CancelEventArgs e)
    {
        Application.Current.Shutdown();
    }
}

Since there should only be one Editor, I made it to a singleton class using the Lazy class. And I used the Visibility property to reuse the Window. Lastly, by registering an event handler to the Closing event of Application.Current.MainWindow, MyEditor can be exit immediately through the Shutdown() method.

MyEditor class is not implemented with MVVM. View, ViewModel, and Model were created separately, and MyEditor class refers to these and uses them.

Now MyEditor works as I want but, I wonder if this approach is the right way. Or is there a problem I'm not aware of?

Answer 1

By making MyEditor a Singleton, you have decided that there must not ever be more than one editor window. The big question is, is that decision backed by a requirement for the application.

If there is no such requirement, it is just a matter of time before someone comes up with a nice idea that requires the creation of 2 editor windows and then your nice Singleton design completely breaks down.

The Singleton design pattern is often seen as one of the most mis-used design patterns. People don't seem to realize there is a fundamental difference between "I currently have/need only one" and "there can never be more than one". The Singleton design pattern is specifically meant to address that second case and even then it is not without issues because Singleton classes are notorious trouble points for unittests.

Answer 2

Even if you know for sure that there will never be the need fore more than one MyEditor windows - the question you should ask yourself here is:

Does this simplify the code? - Or does it complicate things for no real benefit?

For example, if Instance might be accessed from different places in the code, or maybe nowhere, and you don't know beforehand if or in what order this property will be accessed, then using Lazy is probably a simple and straightforward solution.

However, from what you wrote I would expect the place where MyEditor is created is exactly in the Main function, and nowhere else. Then instead of using Lazy, you could simply provide something like an InitInstance method. The resulting number of code lines will probably be not be very different, but it does make things more explicit, straightforward and will still leave the room open for instantiating a second instance of MyEditor in case this will ever become necessary.