C# – Is it good practice to use user controls to structure WPF forms even if these user controls are only used once

I've sorted it out and in the end it went pretty well so I'll give here my recommendations according this painful experience.

(these tips apply for WPF)

• Validation : use INotifyDataErrorInfo instead of rules in XAML, your validation takes place in the object instead and even it's sort of complicate to setup it is really worth it. This interface is appropriate for validation of a complex object. Here's an excellent article showing it : http://anthymecaillard.wordpress.com/2012/03/26/wpf-4-5-validation-asynchrone/ I'd suggest you to not copy/paste it but write it manually. (I did and it took me less time than copy/paste and trying to grasp what it does later)

• Updating your object whenever user inputs values : it's clearly better to do nothing during controls events such as TextChanged but to stick to validation as mentioned above. Hook to Validation.Error events of your controls for managing the logic of your API (in my case when 'name' is set the user cannot search by 'familiarity' so I disabled appropriate controls).

• That leds to a search button that gets enabled only when all values are correct and this is actually the place where you'd grab all params and your stuff, all in one place, not scattered anymore through events of controls.

• Also, do not hesitate to use a middle-man object even though it won't contain all the fields of the original object it still proved to be helpful as that's where I implemented INotifyDataDataErrorInfo. There was absolutely no way that I modify the source object which is fine and part of a library anyway ! For multiple choices a user can do on a ListBox, I didn't use it, I just populated them and that's it. (they are all valid by nature, in my case)

Now a few screenshots and explanations.

Here's the API, not about 2 or 3 fields but 21, when I saw it first I knew it would be a pain to implement but still I underestimated it ...

Here the search button gets enabled only when validation is correct,

Using an extremely simple method, luckily in my case only these 3 cases were needed.

    private void ToggleSearch()
    {
        var b1 = GetHasErrorPropertyValue(TextBoxArtistLocation);
        var b2 = GetHasErrorPropertyValue(TextBoxDescription);
        var b3 = GetHasErrorPropertyValue(TextBoxName);
        ButtonSearch.IsEnabled = !(b1 || b2 || b3);
    }

    private static bool GetHasErrorPropertyValue(DependencyObject dependencyObject)
    {
        if (dependencyObject == null) throw new ArgumentNullException("dependencyObject");
        return (bool)dependencyObject.GetValue(Validation.HasErrorProperty);
    }

    // Hooking to Validation.Error attached property, these boxes all point to here.
    private void TextBox_OnError(object sender, ValidationErrorEventArgs e)
    {
        ToggleSearch();
    }

Here's the output of the query, well the only bothering stuff compared to previous version is that for testing I have to search every time to see the result of the built query for which I assigned Alt-S as a time-saver. Previously I was fetching the result on each value changed in each control, while it looked simple as these were 5 lines methods it soon went to a nightmare. With the new method, I have one hand setting the fields with mouse the other pressing Alt-S and obviously a much simpler class to understand/use/debug.

Last screen : as you can see there isn't much in it, the 3 top methods each call one of the methods below, I think they are self-explanatory. The helper methods are used for setting/retrieving values from controls and to form the query.

Last two samples :

Populating data :

    private async Task PopulateData()
    {
        string apiKey = App.GetApiKey();

        var years = GetDescribedEnum<ArtistSearchYear>().ToList();
        ComboBoxArtistEndYearAfter.ItemsSource = years;
        ComboBoxArtistEndYearBefore.ItemsSource = years;
        ComboBoxArtistStartYearAfter.ItemsSource = years;
        ComboBoxArtistStartYearBefore.ItemsSource = years;

        var rankTypes = GetDescribedEnum<ArtistSearchRankType>().ToList();
        ComboBoxRankType.ItemsSource = rankTypes;

        var results = Enumerable.Range(0, 101).Select(s => s.ToString(CultureInfo.InvariantCulture)).ToList();
        results.Insert(0, string.Empty);
        ComboBoxResults.ItemsSource = results;

        var sort = GetDescribedEnum<ArtistSearchSort>().OrderBy(s => s.Description).ToList();
        ComboBoxSort.ItemsSource = sort;

        var start = Enumerable.Range(0, 3).Select(s => (s * 15).ToString(CultureInfo.InvariantCulture)).ToList();
        start.Insert(0, string.Empty);
        ComboBoxStart.ItemsSource = start;

        var buckets = GetDescribedEnum<ArtistSearchBucket>().OrderBy(s => s.Description).ToList();
        ListBoxBuckets.ItemsSource = buckets;

        var genres = await Queries.ArtistListGenres(apiKey);
        ListBoxGenre.ItemsSource = genres.Genres.Select(s => s.Name);

        var styles = await Queries.ArtistListTerms(apiKey, ArtistListTermsType.Style);
        ListBoxStyle.ItemsSource = styles.Terms.Select(s => s.Name);

        var moods = await Queries.ArtistListTerms(apiKey, ArtistListTermsType.Mood);
        ListBoxMood.ItemsSource = moods.Terms.Select(s => s.Name);
    }

Retrieving parameters :

    private void RunSearch()
    {
        var parameters = new ArtistSearchParameters();

        var year = (Func<ComboBox, ArtistSearchYear?>)((c) =>
            {
                if (c.SelectedValue == null) return null;
                var value = GetDescribedObjectValue<ArtistSearchYear>(c.SelectedValue);
                return value == ArtistSearchYear.Invalid ? (ArtistSearchYear?)null : value;
            });
        parameters.ArtistEndYearAfter = year(ComboBoxArtistEndYearAfter);
        parameters.ArtistEndYearBefore = year(ComboBoxArtistEndYearBefore);
        parameters.ArtistStartYearAfter = year(ComboBoxArtistStartYearAfter);
        parameters.ArtistStartYearBefore = year(ComboBoxArtistStartYearBefore);

        if (ComboBoxRankType.SelectedValue == null)
        {
            parameters.RankType = null;
        }
        else
        {
            var value = GetDescribedObjectValue<ArtistSearchRankType>(ComboBoxRankType.SelectedValue);
            parameters.RankType = value == ArtistSearchRankType.Invalid ? (ArtistSearchRankType?)null : value;
        }

        var selectedValue = ComboBoxSort.SelectedValue;
        if (selectedValue == null)
        {
            parameters.Sort = null;
        }
        else
        {
            var value = GetDescribedObjectValue<ArtistSearchSort>(selectedValue);
            parameters.Sort = value == ArtistSearchSort.Invalid ? (ArtistSearchSort?)null : value;
        }

        parameters.Start = StringToNullableInt(GetSelectorValue<string>(ComboBoxStart));
        parameters.Results = StringToNullableInt(GetSelectorValue<string>(ComboBoxResults));

        parameters.MaxFamiliarity = SliderMaxFamiliarity.Value < 1.0d ? (double?)SliderMaxFamiliarity.Value : null;
        parameters.MinFamiliarity = SliderMinFamiliarity.Value > 0.0d ? (double?)SliderMinFamiliarity.Value : null;
        parameters.MaxHotttnesss = SliderMaxHottness.Value < 1.0d ? (double?)SliderMaxHottness.Value : null;
        parameters.MinHotttnesss = SliderMinHottness.Value > 0.0d ? (double?)SliderMinHottness.Value : null;

        if (CheckBoxFuzzyMatch.IsChecked.HasValue)
            parameters.FuzzyMatch = CheckBoxFuzzyMatch.IsChecked.Value ? (bool?)true : null;
        if (CheckBoxLimit.IsChecked.HasValue)
            parameters.Limit = CheckBoxLimit.IsChecked.Value ? (bool?)true : null;

        var buckets =
            GetSelectedItems<DescribedObject<ArtistSearchBucket>>(ListBoxBuckets)
                          .OrderBy(s => s.Description)
                          .Select(s => s.Value)
                          .ToList();
        parameters.Buckets = buckets.Count > 0 ? buckets : null;
        var genres = GetSelectedItems<string>(ListBoxGenre).OrderBy(s => s).ToList();
        parameters.Genres = genres.Count > 0 ? genres : null;
        var moods = GetSelectedItems<string>(ListBoxMood).OrderBy(s => s).ToList();
        parameters.Moods = moods.Count > 0 ? moods : null;
        var style = GetSelectedItems<string>(ListBoxStyle).OrderBy(s => s).ToList();
        parameters.Styles = style.Count > 0 ? style : null;

        parameters.ArtistLocation = string.IsNullOrEmpty(TextBoxArtistLocation.Text)
                                        ? null
                                        : TextBoxArtistLocation.Text;
        parameters.Description = string.IsNullOrEmpty(TextBoxDescription.Text)
                                     ? null
                                     : Regex.Split(TextBoxDescription.Text, @", ?")
                                            .Where(s => !string.IsNullOrWhiteSpace(s))
                                            .ToList();
        if (string.IsNullOrEmpty(TextBoxName.Text))
        {
            parameters.Name = null;
        }
        else
        {
            parameters.Name = TextBoxName.Text;
            parameters.MaxFamiliarity = null;
            parameters.MaxHotttnesss = null;
            parameters.MinFamiliarity = null;
            parameters.MinHotttnesss = null;
        }

    }

In my case nullables were vital as they allowed me to discard values when building the query.

For building the query I cheated somehow using JSON :

    protected static string GetUrlParameters(string json)
    {
        if (json == null) throw new ArgumentNullException("json");
        JsonTextReader reader = new JsonTextReader(new StringReader(json));
        string path = string.Empty;
        List<string> list = new List<string>();
        while (reader.Read())
        {
            JsonToken type = reader.TokenType;
            if (type == JsonToken.PropertyName)
            {
                path = reader.Path;
            }
            else
            {
                bool b0 = type == JsonToken.Integer;
                bool b1 = type == JsonToken.Float;
                bool b2 = type == JsonToken.String;
                bool b3 = type == JsonToken.Boolean;
                if (b0 || b1 || b2 || b3)
                {
                    var value = reader.Value.ToString();
                    if (b3) value = value.ToLower();

                    string item = string.Format("{0}={1}", path, value);
                    list.Add(item);
                }
            }
        }
        return string.Join("&", list);
    }

DescribedObject is a middle-man object that holds the desired value and the description is fetched from a value [DescriptionAttribute]

public class DescribedObject 
{
    private readonly string _description;
    private readonly Object _value;

    public DescribedObject() // NOTE : for design-time
    {

    }
    protected DescribedObject(Object value, string description)
    {
        _value = value;
        _description = description;
    }

    public string Description
    {
        get { return _description; }
    }

    public Object Value
    {
        get { return _value; }
    }

    public override string ToString()
    {
        return string.Format("Value: {0}, Description: {1}", _value, _description);
    }
}

public class DescribedObject<T> : DescribedObject
{

    public DescribedObject(T value, string description)
        : base(value, description)
    {
    }
    public new T Value
    {
        get
        {
            return (T)base.Value;
        }
    }
}

A described object :

[JsonConverter(typeof(PropertyEnumConverter))]
public enum ArtistSearchYear
{
    [Description(null), JsonProperty(null)]
    Invalid = 0,
    [Description("1970"), JsonProperty("1970")]
    Year1970,
    [Description("1971"), JsonProperty("1971")]
    Year1971
}

(PropertyEnumConverter is a simple object that will convert these values according their JsonProperty attribute.)

So as you can see, I kept it stupidly simple but it took me some time as at first I had a more complex approach, again the simplicity proven really helpful though it sounds stupid sometimes.

I haven't talked about the XAML side, briefly there's absolutely nothing in it beside controls and templates, only bindings have these two properties set for them to validate. (also, the object implements INotifyPropertyChanged)

<TextBox x:Name="TextBoxArtistLocation"
            Style="{StaticResource TextBoxInError}"
            Validation.Error="TextBox_OnError"
            Text="{Binding ArtistLocation, NotifyOnValidationError=True, UpdateSourceTrigger=PropertyChanged}" />

I hope this answer will help someone, I've tried to be as concise as possible but if you have any question/comment, ask below.

I've found that there is way more repetition of code in a WebForms application than is immediately noticeable. I've been banging my head with this exact problem, and I've come up with several remedies.

DRY Up Your UserControls (And Models, Too!)

When you look at each individual page, you see so many differences that you can't possibly imagine code is being repeated. Look at the data you are saving to the database. On an application I work on, we have about 10 different forms all saving "people" records to the database with the same fields, but different fields are available in each view. At first I thought there was too much custom code, but then I realized it's all the same data in the same table. I created a generic User Control to edit "people" objects -- Populate form fields based on a domain model, and repopulate the domain model based on the form fields.

I also created a view-model that encompassed display logic, and then use that to affect the user control.

Person Domain Model

public enum PersonType
{
    Applicant = 1,
    Foo = 2
}

public class Person
{
    public int Id { get; set; }
    public string FirstName { get; set; }
    public string LastName { get; set; }
    public string TaxId { get; set; }
    public IEnumerable<PersonType> Types { get; private set; }

    public bool IsApplicant
    {
        get { return Types.Contains(PersonType.Applicant); }
    }

    public bool IsSomethingElse
    {
        get { return Types.Contains(PersonType.Foo); }
    }

    public bool RequiresTaxId
    {
        get { return IsApplicant; }
    }
}

Person View-Model

public class PersonViewModel
{
    public Person Person { get; private set; }

    public bool ShowTaxIdField { get { return Person.RequiresTaxId; } }

    public PersonViewModel(Person person)
    {
        Person = person;
    }
}

(A Snippet Of) The User Control

<p id="TaxIdField" runat="server">
    <asp:Label ID="TaxIdLabel" runat="server" AssociatedControlID="TaxId">Tax Id:</asp:Label>
    <asp:TextBox ID="TaxId" runat="server" />
</p>

The C# Code-Behind

public partial class PersonUserControl : System.Web.UI.UserControl
{
    public void SetModel(PersonViewModel model)
    {
        TaxId.Text = model.Person.TaxId;
        TaxIdField.Visible = model.ShowTaxIdField;
        // ...
    }

    public PersonViewModel GetModel()
    {
        var model = new PersonViewModel(new Person()
        {
            TaxId = TaxId.Text.Trim(),
            // ...
        });

        return model;
    }
}

Push Business Logic Into Your Domain Models

Removing duplicated code also means moving business logic into your domain models. Does this "person" require a tax Id? Well, the Person class should have a property or method called IsTaxIdRequired. Watch Crafting Wicked Domain Models for some additional ideas.

But I Use DataSets...

Stop! Immediately. You are intimately tying your C# code to the underlying database schema. Even if you have to hand write a mapping layer between DataSet objects and your domain models, you'll still be further ahead because now you can bundle behavior and business rules with your data in one neat, and tidy class. By using DataSets, your Design and Code-Behind files implement business logic! Stop this as soon as you can. Even though the WebForms framework doesn't strictly adhere to the Model-View-Controller pattern, the C# Code-Behind should be viewed as the "Controller" and the Design file should be seen as the "View". The only thing left is the "Model", which should not be a DataSet. DataSet objects give you data, but no behavior leading to repeated business logic.

Use Helper Classes For Initializing the User Interface

All to often I see this in 15 different UserControls:

public InitStateDropdownList()
{
    ddlState.DataSource = ...
    ddlState.DataBind();
    ddlState.Items.Insert(0, new ListItem("Select", string.Empty));
}

Push this into a helper class:

public static class DropDownListHelper
{
    public static void InitStates(DrowDownList list)
    {
        list.DataSource = // get options from database or session
        list.Items.Insert(0, new ListItem("Select", string.Empty));
        list.DataBind();
    }
}

And use it:

DropDownListHelper.InitStates(ddlState);

Some people rile against "helper" classes, but it's surely better than writing what is essentially the same 3 lines of code in multiple places. Even if you can't create super generic User Controls, at least weed out the repetitive code that initializes components upon page load.

When is the BIG Rewrite the Answer?

Almost never. For all my complaints about the WebForms framework, it at least compartmentalizes pages and forms, which is a great setup for a long term refactoring job. The key is adding test coverage. I've had a lot of success using CodedUI Tests and SpecFlow for testing WebForms applications, since most if not all business logic is scattered between C# and ASP in 19 different files. At least write some tests to validate existing business rules don't get broken during the many refactoring sessions you have in your future.