WPF best practices


  • Banned

    New dilemma: what's the idiomatic way to open standard Windows dialogs (file/folder selection in particular)? I can only find WinForms version, and they're inherently anti-MVVM - by which I mean I cannot data-bind to viewmodel object that the main viewmodel could hold, because the data are dialog window's properties.



  • @gąska said in WPF best practices:

    New dilemma: what's the idiomatic way to open standard Windows dialogs (file/folder selection in particular)? I can only find WinForms version, and they're inherently anti-MVVM - by which I mean I cannot data-bind to viewmodel object that the main viewmodel could hold, because the data are dialog window's properties.

    I think that's where databinding stops and you write the old fashioned imperative code to open the dialog and get back the file.



  • @gąska In my (unfinished) WPF application, I have it bound as follows:

    XAML (in MainWindow.xaml):

    <Window.InputBindings>
            <KeyBinding Command="{Binding SaveCharacterCommand}" Key="S" Modifiers="Ctrl"/>
            <KeyBinding Command="{Binding OpenCharacterCommand}" Key="O" Modifiers="Ctrl"/>
            <KeyBinding Command="{Binding NewCharacterCommand}" Key="N" Modifiers="Ctrl"/>
        </Window.InputBindings>
    

    C# (in a view model that serves for the window shell):

    public ICommand OpenCharacterCommand
            {
                get { return new Views.DelegateCommand<object>(OpenCommandExecute); }
            }
    ...
    public void OpenCommandExecute(object sender)
            {
                var location = Views.WindowManager.SelectExistingFile();
                if (location == null) { return; } //user canceled open dialog
                currentCharacterPath = location;
                var serializer = new DataContractSerializer(typeof(ICSheetCore.Data.CharacterData));
                FileStream reader = new FileStream(location, System.IO.FileMode.Open);
                var cData = (ICSheetCore.Data.CharacterData)serializer.ReadObject(reader);
                reader.Close();
                var builder = new CharacterFactory(cData.Name, spellDB, featureFactory);
                currentCharacter = builder.BuildFromStoredData(cData);
                setViewModels();
                SetWindowTitle(false);
            }
    

    Where DelegateCommand is https://github.com/bentomhall/ICSheet/blob/Core/ICSheet5e/Views/DelegateCommand.cs

    and the relevant code from Views.WindowManager.SelectExistingFile() is

    public static string SelectExistingFile()
            {
                Microsoft.Win32.OpenFileDialog dlg = new Microsoft.Win32.OpenFileDialog();
                dlg.FileName = "Character";
                dlg.DefaultExt = ".dnd5e";
                dlg.Filter = "5th Edition Character Sheets (.dnd5e)|*.dnd5e";
    
                var result = dlg.ShowDialog();
                if (result == true)
                {
                    return dlg.FileName;
                }
                else
                {
                    return null;
                }
            }
    

    I hope some of that helps. And isn't awful practice.


  • Discourse touched me in a no-no place

    @benjamin-hall said in WPF best practices:

            if (result == true)
    

    😶



  • @dkf Yeah, looking at that makes me queasy. It's been a while since I wrote that.



  • @benjamin-hall said in WPF best practices:

    @gąska In my (unfinished) WPF application, I have it bound as follows:

    C# (in a view model that serves for the window shell):

    public void OpenCommandExecute(object sender)
            {
                var location = Views.WindowManager.SelectExistingFile();
    

    But that indicates your View Model has knowledge of the View(s)! [at least a static method that is associated with the views.....



  • @thecpuwizard Views is a namespace in this case. The awkwardly-named ViewManager class would (under better design) be injected into the view model. Here it's a utility class with only static methods that knows how to open the dialog. It's not really a view per se, but it's related to the presentation. It's separate from the view that's connected to this view model--it only exists to open ancillary windows and pass the data back to the view models.

    As I understand it, that's close enough to MVVM for most purposes. The view is bound only to a command, the view model goes and does the rest. It doesn't care how the dialog gets opened, just that it gets a filename to open.



  • @benjamin-hall -

    Presume your Views and ViewModels are in separate assemblies [something I strongly encourage to constrain referencing. WindowManager can not be in the Views assembly (it would trigger circular reference), nor can it be in the ViewModel assembly as it would tie the VM's to a specific view technology...

    Injection is a fine idea. There are a number of ways. Let the ViewModel take a delegate, which is set by the View. This means that the ViewModel needs an accessible Property (or Field - yuck) along with the ability to invoke it....

    Topologically, having an EventHandler in the ViewModel and registering the delegate with "handler+=implementation" is identical to model.SetHandler(implementation)....



  • @thecpuwizard said in WPF best practices:

    @benjamin-hall -

    Presume your Views and ViewModels are in separate assemblies [something I strongly encourage to constrain referencing.

    I don't understand this. ViewModels are inherently tied to specific technologies--there's no way to reuse a WPF viewmodel in an iOS app or a web-based presentation layer. Models? Sure. ViewModels? Those have to be built knowing what's going to be presented. Different view technologies need different pieces presented in different ways.

    But no, in the code I have they're in the same assembly--all my WPF presentation is. The core pieces (data persistence, models, business logic) is all in a separate one because that's cross-platform.

    WindowManager can not be in the Views assembly (it would trigger circular reference), nor can it be in the ViewModel assembly as it would tie the VM's to a specific view technology...

    I don't see how you'd get circular reference from that code. Explain? It's an entirely static class that does Win32-based things (opens dialogs, returns the result). It doesn't know about the view models--it'll answer any request anybody sends it. There may be a using in there, but I'm pretty sure it's unused (don't have VS on this machine to test).

    Injection is a fine idea. There are a number of ways. Let the ViewModel take a delegate, which is set by the View. This means that the ViewModel needs an accessible Property (or Field - yuck) along with the ability to invoke it....

    Topologically, having an EventHandler in the ViewModel and registering the delegate with "handler+=implementation" is identical to model.SetHandler(implementation)....

    As far as I'm aware, the binding to a command in the view and letting the view model/delegate infrastructure handle the actual work is the "proper" way of handling interactions with a WPF dialog. Same with opening a sub-window. The only piece of code that interacts with the new windows is the viewmodel that handles the pieces of the application that don't belong to a particular set of data--basically the parts under File in the top menu, as well as bootstrapping the rest of the viewmodels. It's only barely a viewmodel itself--it acts more like application bootstrap code.

    BTW the whole repo is at https://github.com/bentomhall/ICSheet if anyone wants to tear me apart. It's not in active development, since I found a better way of doing that task using pre-existing apps.



  • @benjamin-hall

    First the easy part. If the View and ViewModels are in different assemblies, your approach would require View to reference ViewModel [normal] but also ViewModel to reference View [to get to the manager].

    I disagree strongly that ViewModels are in any way tied to the presentation technology. As you stated, they control what do display, and should be independent of how to display it. I commonly use the same VM's for different technologies [WinForm, WPF, UWP, non-Microsoft].

    I do agree that if the content being presented changes [perhaps different UI's for different form factors] then distinct VM's are used.

    As a note, (and granting the the "View" part of ViewModel is not proper nomenclature in these cases) having non-UI "Views" for testing and even "service" type interactions has many useful applications.

    I prefer having many smaller assemblies (e.g. one for Models, one for Persistence, one for Business Logic, etc.) simply because it provides better application of the "internal" access level. Years ago [2004?]a bunch of us advocated for "Namespace Scoped Internals" but the concept never gained sufficient traction to become a language feature.



  • @gąska Welcome to The Daily WPF! Ehm WTF, ..., ehm, where is the difference actually?
    When the WPF team created it, they did not at all think of architectural questions. They thought of fancy controls. Yes, everything must look great and fancy, with all those animations and crap. And every beginner should be able to do so.
    Do not forget: the members of that team came from the VisualBasic team. So "Binding" works with On Error Resume Next - it must not really fail when something goes wrong, just try the next item. It is so much first class programming - and with "first class" you should not think of airplane travel or hotels, but of elementary school.
    Later on, architects had to add another layer. Since Model-View-SomethingInBetween was in fashion at that time, they decided for MVVM.
    So when a senior developer starts learning WPF, he'll stumble upon many things to drive him nuts.

    As for the Cancel button, that's pretty easy, if there is nothing else to do than closing the window:
    Button Content="_Close" IsCancel="True"
    Nothing required in the ViewModel.

    Spawning a new dialog is ...
    Well, Prism introduced a method for that which I do not know.
    I usually create an "InteractionService" interface, and a concrete implementation for WPF (and maybe some more for tests).
    When the ViewModel get created, the concrete interaction service is injected. The viewmodel must not create it with a new WpfInteractionService();, otherwise the viewmodel becomes dependend on that specific view.
    In the InteractionService interface you can e.g. define
    bool SelectFile(ref string filename, string filter);
    And implement it in the WpfInteractionService

    public bool SelectFile(ref string filename, string filter)
    {
        OpenFileDialog ofd = new OpenFileDialog
        {
            Multiselect = false,
            FileName = filename,
            Filter = filter
        };
    
        if (ofd.ShowDialog() == true)
        {
            filename = ofd.FileName;
            return true;
        }
        return false;
    }
    

    And call it in your ViewModel:

    private void SelectFile()
    {
        // IInteractionService service was injected
        string tmp = _Filename; // current value of backing field
        if (service.SelectFile(ref tmp, Item))
        {
            Filename = tmp; // informs the view via PropertyChanged event
        }
    }
    

    Well, proper placement is another issue...

    Take care, WPF is not at all a mature technology. Just look at https://stackoverflow.com/questions/11341853/accept-button-and-data-binding-inconsistency or the many WTFs you may encounter when it comes to floating point number which ought to be shown with a decimal comma...



  • @berniethebernie - Interesting, I worked for Microsoft around that time. My memories of the meetings related to WPF (I was actually more involved with WCF and WF, but still) are quite different.

    In terms of Binding Errors, it is completely configurable (with many options). I do remember the debates about the default of simply outputting a warning as part of the WPF Trace (I was not in favor of the decision).

    One example of changing this to be an exception can be found on GitHub: https://github.com/bblanchon/WpfBindingErrors



  • @thecpuwizard Nice to hear a comment from someone with first-hand-knowledge. My post is based on internet rumors with some added WTF. 😀

    That WpfBindingErrors extension is a great help. I discovered it someone, and use it since. It is not part of the Microsoft components (they also missed ViewModelBase, RelayCommand, and several other components which everyone has to copy from some web sources...)

    By the way, I think that WCF has also some WTFs: why do I need to add a ServiceKnownType attribute for every single concrete class I'd like to transport somewhen over the web? That is, those types must be known at compile time. Consequently, I must place the interface for the service in some dll which references all those dlls with the concrete types. And I cannot add a new type later on without re-compiling.

    I do not want to blame you for these shortcomings. But I'd like to say that Microsoft did a lot of bad design/architecture with WPF, WCF, WF: best to place a T between the W and the F.



  • @berniethebernie - Just to be clear, I was there, but I was not really involved in the decisions - more of a stakeholder (consumer of their code) role...

    That being said, no extensible system is going to contain every feature that every end user desires. In fact the reason for an extensible system is to reduce what needs to be provided by the original vendor and allow the community/ecosystem/customer to create the extensions.

    PresentationTraceSources is part of the core WPF, and at the end of the day, WpfBindingErrors is nothing more (or less) than a sample [albeit a very useful sample] of what can be done with a listener.

    As for ServiceKnownType, you are incorrect in "for every single concrete class I'd like to transport somewhen". IOT is only needed for types that are not apparent from the service contract. There are many good reasons for this, which I would he happy to discuss elsewhere.

    Were there WTFs in W*F? Sure! But given the scope of the efforts I do not see an overt amount.



  • @dkf said in WPF best practices:

    @benjamin-hall said in WPF best practices:

            if (result == true)
    

    😶

    That's a nullable boolean (bool?) so it can be true, false or null.


  • And then the murders began.

    @marczellm Better to use if (result.GetValueOrDefault()) - you both avoid causing other devs confusion when they ask ":wtf: is someone doing an explicit == true?", and you avoid unnecessary boxing.


  • Dupa

    @gąska said in WPF best practices:

    New dilemma: what's the idiomatic way to open standard Windows dialogs (file/folder selection in particular)? I can only find WinForms version, and they're inherently anti-MVVM - by which I mean I cannot data-bind to viewmodel object that the main viewmodel could hold, because the data are dialog window's properties.

    Write a good ol' wrapper for the dialog. This way you'll be able to write tests for the VM. ;)



  • @kt_ said in WPF best practices:

    @gąska said in WPF best practices:

    New dilemma: what's the idiomatic way to open standard Windows dialogs (file/folder selection in particular)? I can only find WinForms version, and they're inherently anti-MVVM - by which I mean I cannot data-bind to viewmodel object that the main viewmodel could hold, because the data are dialog window's properties.

    Write a good ol' wrapper for the dialog. This way you'll be able to write tests for the VM. ;)

    Here is a nice solution (not without problems, but still nice): https://www.codeproject.com/Articles/1236588/File-System-Controls-in-WPF-Version-III


  • Banned

    @thecpuwizard said in WPF best practices:

    nice solution

    codeproject.com

    It's not. I haven't read the article yet, but I'm sure as hell it's not nice. Copy-pasting random code found online directly into your project cannot be nice by definition. Now, if it was made into proper library, nicely packed up and uploaded to NuGet, it would be entirely different conversation.



  • @gąska said in WPF best practices:

    @thecpuwizard said in WPF best practices:

    nice solution

    codeproject.com

    It's not. I haven't read the article yet, but I'm sure as hell it's not nice. Copy-pasting random code found online directly into your project cannot be nice by definition. Now, if it was made into proper library, nicely packed up and uploaded to NuGet, it would be entirely different conversation.

    100% agreement about "Copy-pasting random code found online directly into your project cannot be nice by definition.".. Alas, I believe the same is largely true about including "random" packages found on NuGet.

    Did you read the article, the discussion on the design is fairly deep. i have used derivative works of this (with attribution) a number of times. Some of the lines of code made it from the original example (there are only so many ways to do certain things), but the overall codebase was brought into alignment with the overall design of my application(s).


  • Banned

    @thecpuwizard said in WPF best practices:

    Alas, I believe the same is largely true about including "random" packages found on NuGet.

    Even the most shitty package has huge benefits: there's always only one copy linked, the namespace and class names are always the same in all projects, it works the same everywhere, and it's completely isolated from your codebase.



  • @gąska said in WPF best practices:

    most shitty package

    My vote was for an HTML optimization package [now delisted] that did a good job of what it represented itself to do, but also internally used reflection and other techniques to data mine within the application and then expose the information via multiple channels. At least 3 significant commercial breaches were tracked (at least in part) to this package being used...

    Not exactly what i would refer to as "huge benefits" (at least from the impacted parties POV, I am pretty sure it provided them to the originator - don't know if the person was ever actually caught)


  • Banned

    @thecpuwizard that's not copy-pasting vs. packaging debate, that's closed vs. open source debate.



  • @gąska said in WPF best practices:

    @thecpuwizard that's not copy-pasting vs. packaging debate, that's closed vs. open source debate.

    Agreed, but you previously posted

    Now, if it was made into proper library, nicely packed up and uploaded to NuGet, it would be entirely different conversation.

    Which does not intrinsically mean that source would be available. :) :)


  • Banned

    @thecpuwizard it's .NET. All the source is available. Kinda.


Log in to reply