Multi-checkbox validation

I wrote the following custom validation annotation for my project to confirm that at least one checkbox of a group of checkboxes is checked:

public class ValidateAtLeastOneChecked : ValidationAttribute {
    public string[] CheckBoxFields {get; set;}
    public ValidateAtLeastOneChecked(string[] checkBoxFields) {
        CheckBoxFields = checkBoxFields;
    }

    protected override ValidationResult IsValid(Object value, ValidationContext context) {
        Object instance = context.ObjectInstance;
        Type type = instance.GetType();

        foreach(string s in CheckBoxFields) {
            Object propertyValue = type.GetProperty(s).GetValue(instance, null);
            if (bool.Parse(propertyValue.ToString())) {
                return ValidationResult.Success;
            }
        }
        return new ValidationResult(base.ErrorMessageString);
    }
}

I then use it like this:

[ValidateAtLeastOneChecked(new string[] { "Checkbox1", "Checkbox2", "Checkbox3", "Checkbox4" }, ErrorMessageResourceType=typeof(ErrorMessageResources),ErrorMessageResourceName="SelectAtLeastOneTopic")]
public bool Checkbox1{ get; set; }
public bool Checkbox2{ get; set; }
public bool Checkbox3{ get; set; }
public bool Checkbox4{ get; set; }

The aim was to get the check boxes all validating and keep the validations and their localization contained in the ViewModel. This is working, my only concern is that I expected to find an easy google-paste piece of code for this and did not... Am I missing something easier?

As a secondary question I am attaching this to only one checkbox field, which I think is fine; but is there any unintended consequences of that (other than that any css error highlighting would only apply to the one checkbox)?

I'll answer your second question first! The fact that one of your your checkboxes needs to be checked is a validation that is more about your class than one of your checkbox. I mean, individually, a checkbox doesn't need to be checked, but your model requires one of them to be. Which is why I would put the validation attribute on your class instead of on one of the checkbox. Then, to show your error message on your client HTML you would need to set Html.ValidationSummary(true) in your view.

For your first questions, there a lot of possibles way to do what you did, but your way is fine I believe. I'll propose another one though.

I think you should make another Model that would only contains your checkboxes. A CheckBoxList or something that is more appropriate in your context (that I know nothing about). Then you could use reflection to get all checkboxes and check if one is checked.

It would look like this :

public class ValidateAtLeastOneCheckedAttribute : ValidationAttribute
{
   protected override ValidationResult IsValid(object value, ValidationContext validationContext)
   {
      Type type = value.GetType();
      IEnumerable checkBoxeProperties = type.GetProperties(BindingFlags.Instance | BindingFlags.Public).Where(p => p.PropertyType == typeof (bool));

      foreach (PropertyInfo checkBoxProperty in checkBoxeProperties)
      {
         var isChecked = (bool)checkBoxProperty.GetValue(value);
         if (isChecked)
         {
            return ValidationResult.Success;
         }
      }

      return new ValidationResult(base.ErrorMessageString);
   }
}

[ValidateAtLeastOneChecked]
public class CheckBoxList
{
   public bool Checkbox1 { get; set; }
   public bool Checkbox2 { get; set; }
   public bool Checkbox3 { get; set; }
   public bool Checkbox4 { get; set; }
   public bool Checkbox5 { get; set; }
}

The advantages of using such a solution is that you don't need to write down every property in a string[], which could lead to errors (if you forgot one or wrote one incorrectly). Also, you don't need to use validationContext.ObjectInstance since the target of your attribute is the instance itself.

I'll review your current code too, because there is three points that I see could be "better" :)

By convention, attributes should end with the suffix Attribute (which I changed already in my example). You won't need to write Attribute when you use it (as shown in my example, which compiles!).

Also, your brackets are following the Java standard, not the C# one. You should put opening brackets on a new line.

Finally, instead of doing this :

Object propertyValue = type.GetProperty(s).GetValue(instance, null);
if (bool.Parse(propertyValue.ToString())) {//...

You can cast the propertyValue to bool directly, it will save you a cast to string.

StackExchange Code Review Q#72160, answer score: 7