ReactiveUI And WPF - Reusing a value to update multiple properties

Question

I've created my first ReactiveUI app in WPF.

Simply, the user enters their full name in one textbox, and then the name gets split into its parts and displayed in the other textboxes. This is just an example ... I realise parsing names is often futile!

MainWindow.xaml:

<Window x:Class="TestHumanName.MainWindow"
        xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
        xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
        Title="MainWindow" Height="392" Width="391">
    <StackPanel Orientation="Vertical">
        <StackPanel Orientation="Horizontal">
            <Label Content="Full" />
            <TextBox Width="100" Text="{Binding Full, UpdateSourceTrigger=PropertyChanged}"/>
            <Button Content="Go"/>
        </StackPanel>
        <StackPanel Orientation="Horizontal">
            <Label Content="Title" />
            <TextBox Width="100" Text="{Binding Title, Mode=OneWay}"/>
        </StackPanel>
        <StackPanel Orientation="Horizontal">
            <Label Content="First" />
            <TextBox Width="100" Text="{Binding First, Mode=OneWay}"/>
        </StackPanel>
        <StackPanel Orientation="Horizontal">
            <Label Content="Middle" />
            <TextBox Width="100" Text="{Binding Middle, Mode=OneWay}"/>
        </StackPanel>
        <StackPanel Orientation="Horizontal">
            <Label Content="Last" />
            <TextBox Width="100" Text="{Binding Last, Mode=OneWay}"/>
        </StackPanel>
    </StackPanel>
</Window>

MainWindow.xaml.cs:

using System.Windows;
using TestHumanName.ViewModel;

namespace TestHumanName
{
    /// <summary>
    /// Interaction logic for MainWindow.xaml
    /// </summary>
    public partial class MainWindow : Window
    {
        public MainWindow()
        {
            InitializeComponent();
            DataContext = new MainViewModel();
        }
    }
}

MainViewModel.cs: (ignore the name parsing bit!)

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
//
using ReactiveUI;
using System.Reactive.Linq;
using System.Text.RegularExpressions;

namespace TestHumanName.ViewModel
{
    public class MainViewModel : ReactiveObject
    {

        public MainViewModel()
        {
            this.WhenAnyValue(x => x.Full).Where(x => x != null).Select(x => ParseName(x).Title)
                .ToProperty(this, x => x.Title, out __oapTitle);
            this.WhenAnyValue(x => x.Full).Where(x => x != null).Select(x => ParseName(x).First)
                .ToProperty(this, x => x.First, out __oapFirst);
            this.WhenAnyValue(x => x.Full).Where(x => x != null).Select(x => ParseName(x).Middle)
                .ToProperty(this, x => x.Middle, out __oapMiddle);
            this.WhenAnyValue(x => x.Full).Where(x => x != null).Select(x => ParseName(x).Last)
                .ToProperty(this, x => x.Last, out __oapLast);
        }

        private string __sFull;
        public string Full
        {
            get { return __sFull; }
            set { this.RaiseAndSetIfChanged(ref __sFull, value); }
        }

        readonly ObservableAsPropertyHelper<string> __oapTitle;
        public string Title { get { return __oapTitle.Value; } }

        readonly ObservableAsPropertyHelper<string> __oapFirst;
        public string First { get { return __oapFirst.Value; } }

        readonly ObservableAsPropertyHelper<string> __oapMiddle;
        public string Middle { get { return __oapMiddle.Value; } }

        readonly ObservableAsPropertyHelper<string> __oapLast;
        public string Last { get { return __oapLast.Value; } }


        //NAME PARSING FUNCTIONALITY BELOW THIS LINE


        public class Name
        {
            public string Title { get; set; }
            public string First { get; set; }
            public string Middle { get; set; }
            public string Last { get; set; }
            public string Suffix { get; set; }
        }

        public Name ParseName(string s)
        {
            Name n = new Name();

            // Split on period, commas or spaces, but don't remove from results.
            List<string> parts = Regex.Split(s, @"(?<=[., ])").ToList();

            // Remove any empty parts
            for (int x = parts.Count - 1; x >= 0; x--)
                if (parts[x].Trim() == "")
                    parts.RemoveAt(x);

            if (parts.Count > 0)
            {
                // Might want to add more to this list
                string[] prefixes = { "mr", "mrs", "ms", "dr", "miss", "sir", "madam", "mayor", "president" };

                // If first part is a prefix, set prefix and remove part
                string normalizedPart = parts.First().Replace(".", "").Replace(",", "").Trim().ToLower();
                if (prefixes.Contains(normalizedPart))
                {
                    n.Title = parts[0].Trim();
                    parts.RemoveAt(0);
                }
            }

            if (parts.Count > 0)
            {
                // Might want to add more to this list, or use code/regex for roman-numeral detection
                string[] suffixes = { "jr", "sr", "i", "ii", "iii", "iv", "v", "vi", "vii", "viii", "ix", "x", "xi", "xii", "xiii", "xiv", "xv" };

                // If last part is a suffix, set suffix and remove part
                string normalizedPart = parts.Last().Replace(".", "").Replace(",", "").Trim().ToLower();
                if (suffixes.Contains(normalizedPart))
                {
                    n.Suffix = parts.Last().Replace(",", "").Trim();
                    parts.RemoveAt(parts.Count - 1);
                }
            }

            // Done, if no more parts
            if (parts.Count == 0)
                return n;

            // If only one part left...
            if (parts.Count == 1)
            {
                // If no prefix, assume first name, otherwise last
                // i.e.- "Dr Jones", "Ms Jones" -- likely to be last
                if (n.Title == "")
                    n.First = parts.First().Replace(",", "").Trim();
                else
                    n.Last = parts.First().Replace(",", "").Trim();
            }

            // If first part ends with a comma, assume format:
            //   Last, First [...First...]
            else if (parts.First().EndsWith(","))
            {
                n.Last = parts.First().Replace(",", "").Trim();
                for (int x = 1; x < parts.Count; x++)
                    n.First += parts[x].Replace(",", "").Trim() + " ";
                n.First = n.First.Trim();
            }

            // Otherwise assume format:
            // First [...Middle...] Last

            else
            {
                n.First = parts.First().Replace(",", "").Trim();
                n.Last = parts.Last().Replace(",", "").Trim();
                for (int x = 1; x < parts.Count - 1; x++)
                    n.Middle += parts[x].Replace(",", "").Trim() + " ";
                if (n.Middle != null) n.Middle = n.Middle.Trim();
            }

            return n;
        }

    }
}

I have to call ParseName() 4 times each time "Full" changes. It feels like I should be able to do something like put the return value of ParseName() into a stream/observable, and then have all the dependent properties use the same return value, rather than calling ParseName() for each property.

I think I'm just lacking some essential terminology to guide me in the right direction!

---

Update I've changed my MainWindow.xaml and MainViewModel.cs as follows, but it still feels wrong!

MainWindow.xaml

<Window x:Class="TestHumanName.MainWindow"
        xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
        xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
        Title="MainWindow" Height="392" Width="391">
    <StackPanel Orientation="Vertical">
        <StackPanel Orientation="Horizontal">
            <Label Content="Full" />
            <TextBox Width="100" Text="{Binding Full, UpdateSourceTrigger=PropertyChanged}"/>
            <Button Content="Go"/>
        </StackPanel>
        <StackPanel Orientation="Horizontal">
            <Label Content="Title" />
            <TextBox Width="100" Text="{Binding NameObject.Title, Mode=OneWay}"/>
        </StackPanel>
        <StackPanel Orientation="Horizontal">
            <Label Content="First" />
        <TextBox Width="100" Text="{Binding NameObject.First, Mode=OneWay}"/>
        </StackPanel>
        <StackPanel Orientation="Horizontal">
            <Label Content="Middle" />
        <TextBox Width="100" Text="{Binding NameObject.Middle, Mode=OneWay}"/>
        </StackPanel>
        <StackPanel Orientation="Horizontal">
            <Label Content="Last" />
        <TextBox Width="100" Text="{Binding NameObject.Last, Mode=OneWay}"/>
        </StackPanel>
    </StackPanel>
</Window>

MainViewModel.cs

public class MainViewModel : ReactiveObject
{
    public MainViewModel()
    {
        this.WhenAnyValue(x => x.Full).Where(x => x != null).Select(x => ParseName(x))
            .ToProperty(this, x => x.NameObject, out __oapName);
    }

    private string __sFull;
    public string Full
    {
        get { return __sFull; }
        set { this.RaiseAndSetIfChanged(ref __sFull, value); }
    }

    readonly ObservableAsPropertyHelper<Name> __oapName;
    public Name NameObject { get { return __oapName.Value; } }

    //NAME PARSING CODE BELOW THIS POINT
    ...
}

For some reason, I'd envisaged multiple properties would be updatable from the same observable/stream/method return value, but in my new ViewModel, I've circumvented the multiple calls by storing the returned object, rather than separately storing each individual property from the returned object.

Did I solve this correctly? Or is there a better way?

---

Update

I've also noticed(!) that I'm doing the {Binding } in MainWindow.xaml, rather than in the constructor of MainWindow.xaml.cs.

Is there any disadvantage to doing it this way?

Answer 1

<TextBox Width="100" Text="{Binding Last, Mode=OneWay}"/>

This strikes me as a confusing UX. If the content isn't meant to be editable, then you shouldn't use a TextBox control; users expect to be able to edit a text box - and yours is editable, but modifying the content won't do anything to the view model... which is awkward. Either leave it a two-way binding and work out the VM logic for it, or make it a Label.

I like that your view has zero code-behind. However I'd find another way to do this:

DataContext = new MainViewModel();

The view itself shouldn't be responsible for actually instantiating its view model; this tight coupling violates the code against abstractions guideline. Instead of directly instantiating the MainViewModel instance, you could leave that responsibility up to the calling code:

var view = new MainWindow { DataContext = new MainViewModel() };

Now, if you're leaving it up to WPF to create the MainWindow instance in the App class, it's not too bad. All I'm saying is, there are other ways - the xaml designer can work off a DesignInstance:

xmlns:vm="clr-namespace:TestHumanName.ViewModel"
xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006" 
xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
mc:Ignorable="d" 
d:DesignHeight="392" d:DesignWidth="392" 
d:DataContext="{d:DesignInstance vm:MainWindowViewModel}">

The d: xml namespace is ignored at runtime, but the xaml designer will use it - by doing that you no longer need to set the view model in the constructor.

<Button Content="Go"/>

It's pretty rare that a simple string suffices as the Content of a Button control. Even when all I need is a caption, I find it easier to maintain when the content is expressed in a more verbose manner:

<Button>
    <StackPanel Orientation="Horizontal">
        <TextBlock Text="Go" />
    </StackPanel>
</Button>

That way if you change your mind and decide to add an icon beside the "Go" caption, all you have to do is, well, add an icon.

Hard-coding strings is fine for prototyping, but makes it quite painful to localize afterward: it's better to have the strings in a resource file from the start, so if/when you localize all that's left to do is, well, localize the strings. Then you can use a value converter or a markup extension to support the easy-to-use .resx resource format. The problem with localization is that it's often an afterthought, something that's "I don't need this"... until you do. And if you haven't set yourself up for it from the start, you're in for a lot of pain. The solution is simple: don't hard-code string captions into your markup - put your strings in a resource file... where they belong anyway.

---

Your naming scheme for private fields is terrible.

private string __sFull;
readonly ObservableAsPropertyHelper<string> __oapTitle;

There is no need for two underscores. Ever. Don't get me wrong - I use an underscore prefix all the time, for every single one of my private fields. I like how it avoids clashes with locals and parameters, and thus avoids being forced to use an otherwise perfectly redundant this qualifier. But two underscores just doesn't look right; there's no reason for having two of them.

Speaking of the this qualifier, its usage is confusing here:

public string Full
{
    get { return __sFull; }
    set { this.RaiseAndSetIfChanged(ref __sFull, value); }
}

If RaiseAndSetIfChanged is a member of the base class (I'm not familiar at all with reactive stuff), then base would have been a more appropriate and useful qualifier to use; this is simply redundant and uninformative here, and since the member isn't in the same class, it's outright misleading.

But the double-underscores and misplaced this qualifiers aren't the worst offender. Hungarian Notation is. I didn't think there still existed programmers writing C# code in 2016 using Hungarian Notation. Seriously - drop that. There is no need to use type-hinting prefixes for any identifier in any .net code (see why).

---

IMO the Name class shouldn't be a nested type under the ViewModel. It's your Model, and it's a completely separate thing from the ViewModel, as hinted by the pattern's name, Model-View-Viewmodel.

As for your concern with calling ParseName 4 times, it looks like your updated code is what doing that with system.reactive might look like - although again, I've never used that namespace before.

public MainViewModel()
{
    this.WhenAnyValue(x => x.Full).Where(x => x != null).Select(x => ParseName(x))
        .ToProperty(this, x => x.NameObject, out __oapName);
}

Seems legit. It might read a little easier like this though:

public MainViewModel()
{
    WhenAnyValue(name => name.Full).Where(name => name != null)
                                   .Select(name => ParseName(name))
                                   .ToProperty(this, vm => vm.Name, out _name);
}

---

(ignore the name parsing bit!)

Why? It's a really interesting bit of code!

You're populating these arrays every time you call the function, but you shouldn't have to:

// Might want to add more to this list
string[] prefixes = { "mr", "mrs", "ms", "dr", "miss", "sir", "madam", "mayor", "president" };

// Might want to add more to this list, or use code/regex for roman-numeral detection
string[] suffixes = { "jr", "sr", "i", "ii", "iii", "iv", "v", "vi", "vii", "viii", "ix", "x", "xi", "xii", "xiii", "xiv", "xv" };

Make them private static IReadOnlyList<string> fields, that will create them only once for the type instead of per call.

Then, I'd try to extract a private method for each member I'm trying to parse out of that string, to up the abstraction level a bit.