2
\$\begingroup\$

Edit: The method call discussed is a public extension method in a shared library. I'd like it to be safe for others to call.

A little background: I'm maintaining an ancient system written in ASP.Net Webforms. There are lots of validation groups for different user controls, and we want to display the error messages from them in modal dialogs at the page level. Rather then deal with the complexities of having a ValidationSummary for each possible validation group, we're just getting all the validation messages from each user control, making one list of validation messages, and displaying the resulting list in a modal. Easy-peasy.

A co-worker was writing the methods to return validation messages from ASP.Net Validator controls nested inside his user controls. He was writing the methods explicitly:

if (!Validator1.IsValid)
{
    messages.Add(Validator1.ErrorMessage);
}
etc...

I decided to be slick and make an extension method to loop over the Controls collection of a given control (including pages and user controls) and just examine each validator encountered. This way I don't have to write and maintain lots of explicit "GetErrorMessages()" implementations. I don't have problems if somebody renames, removes, or adds a validator and forgets to include it explicitly:

public static IEnumerable<string> GetValidatorMessages(this Control control, string validationGroup)
{
    var messages = new List<string>();

    var validator = control as BaseValidator;
    if (validator != null && IsInvalid(validator, validationGroup))
    {
        messages.Add(validator.ErrorMessage);
    }

    if (control.Controls.Any())
    {
        foreach (Control childControl in control.Controls)
        {
            messages.AddRange(GetValidatorMessages(childControl, validationGroup));
        }
    }

    return messages;
}

Still, this feels imperfect. I'm iterating EVERY CONTROL in the given control. If this is a page, we could be talking lots of nested things in a DataGrid, for example.

Is there are faster way to get all the validation messages from a Page or UserControl?

Second Edit: @Abbas got me thinking along the lines of using a generic (see his answer) but I don't want to encourage using this method with unusual input types like DropDownList, RadioButton, or other ASP.Net controls that CAN have children in their Controls collection, but generally should not.

But, thanks again to him, thinking along the lines of evaluating the minimum number of controls possible, I came up with using the control's Page property to reach the Page.Validators property, which is explicitly set when validators are created instantiated rather than recursively looking at children like my original method does. I wound up with something like the following. Called extension methods are included for clarity's sake:

public static IEnumerable<string> GetValidatorMessages(this Control control, string validationGroup)
{
    var messages = new List<string>();

    // If the given control is an invalid validator, get it's error message.
    var validator = control as BaseValidator;
    if (validator != null && validator.IsValid == false)
    {
        messages.Add(validator.ErrorMessage);
    }

    // Get all the validators that are a child of the given control
    var childValidators = control.Page.Validators
        .Cast<BaseValidator>()
        .Where(v => v.IsChildOf(control) && v.ValidationGroup == validationGroup);

    // Return messages from just those that are not valid.
    messages.AddRange(childValidators
        .Where(childValidator => childValidator.IsValid == false)
        .Select(childValidator => childValidator.ErrorMessage));

    return messages;
}

public static bool IsChildOf(this Control control, Control parentControl)
{
    // Recursively seek through the given control's parents until we either null out, or find the given parentControl.
    return control.Parent != null && (control.Parent == parentControl || control.Parent.IsChildOf(parentControl));
}

This solution still uses recursion to determine if each validator is a child of the given control. But it only ever checks controls that are Validators to begin with, since it gets Control.Page.Validators and works from that collection rather than recursively looking at every child of Control and checking to see if the child is a validator.

I feel I get a couple of things here:

1) Much faster than the original in worst cases. 2) Simple API... No need for a generic parameter.

Thoughts?

\$\endgroup\$
1
\$\begingroup\$

You could filter out the type of controls you do not want to iterate. Let's say for example that you only want to get the validationMessages from only a UserControl, you can filter them out of the Controls property by using a bit of LinQ:

public static IEnumerable<string> GetValidatorMessages(this Control control, string validationGroup)
{
    var messages = new List<string>();
    var validator = control as BaseValidator;

    if (validator != null && IsInvalid(validator, validationGroup))
        messages.Add(validator.ErrorMessage);

    foreach (var childControl in control.Controls.OfType<UserControl>())
        messages.AddRange(GetValidatorMessages(childControl, validationGroup));

    return messages;
}

Edit:

I found that in your original code in the method, when making the recursive call, you don't make use of the extension-call:

messages.AddRange(GetValidatorMessages(childControl, validationGroup));

This should/could be:

messages.AddRange(childControl.GetValidatorMessages(validationGroup));

Otherwise, what's the use of making an extension-method? ;)

Anyway... As you stated in your comment, one can indeed not assume that a person would always want to traverse only a UserControl. Thank God for generics! :D You keep your method but you make it a generic one. When calling the method you specify the type of control you want to traverse. Here's some code I came up with. Please mind that I haven't been able to test this as this is just written out of my head:

public static IEnumerable<string> GetValidatorMessages<T>(this Control control, string validationGroup)
    where T : Control
{
    var messages = new List<string>();
    var validator = control as BaseValidator;

    if (validator != null && IsInvalid(validator, validationGroup))
        messages.Add(validator.ErrorMessage);

    foreach (var childControl in control.Controls.OfType<T>())
        names.AddRange(childControl.GetValidatorMessages<T>(validationGroup));

    return names;
} 
\$\endgroup\$
  • \$\begingroup\$ This is a good idea. For my current scope, I could do this. However, this is a public extension method in a shared library. I'd not be comfortable making the assumption that the caller only wants to traverse UserControls. Note: I'm sorry I didn't mention this in the original question. I'll go edit it now. \$\endgroup\$ – Eric Burcham Jul 17 '13 at 17:14
  • \$\begingroup\$ Please check my updated answer for further help. :) \$\endgroup\$ – Abbas Jul 17 '13 at 20:08
  • 1
    \$\begingroup\$ Thanks for the help! Couple of things: 1) Extension methods can be called as a static method or with the extension syntax. For me the standard static call is clearer since I am used to seeing recursive methods called with this syntax. The extension syntax still applies to any outside caller. But honestly for me, either way is fine. 2) Yes, everybody loves generics, but in this case I think it encourages some weird calls, like control.GetValidatorMessages<DropDownList>(groupName); I'm not sure I want to encourage that. You got me thinking though. I'll add a second "Edit" to my question. \$\endgroup\$ – Eric Burcham Jul 17 '13 at 21:41

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.