Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 4 down vote favorite

The model looks like this, with no attributes:

public class PersonalModel : Validatable
{ 
    public string Name { get; set; } 
    public string Email { get; set; }

    public override void Validate(dynamic item)
    {
        if (this.ValidatesPresenceOf(item.Name, "Name is required"))
        { 
            if (((String)item.Name).Length > 5)
            {
                Errors.Add("name is too long less than 5 chars please!");
            }
        } 
    } 
}

and the action looks like this:

    [HttpPost]
    [ValidateAntiForgeryToken]
    public ActionResult PersonalInfo(PersonalModel model)
    {
        if (model.IsValid(model))
        {
            this.FlashInfo("Your Personal info was successfully received");
            return RedirectToAction("ThankYou");
        }
        else
        { 
            ModelState.AddModelError(string.Empty, String.Join("; ", model.Errors.ToArray())) ;   
            return View(model);
        }
    }

This is the baseclass Validatable:

public class Validatable : DynamicObject
{
    //Hooks
    public virtual void Validate(dynamic item) { }

    //Temporary holder for error messages
    public IList<string> Errors = new List<string>();

    public bool IsValid(dynamic item)
    {
        Errors.Clear();
        Validate(item);
        return Errors.Count == 0;
    }


    //validation methods
    public virtual bool ValidatesPresenceOf(object value, string message = "Required")
    {
        if (value == null || String.IsNullOrEmpty(value.ToString())) 
        {
            Errors.Add(message);
            return false;
        }
        return true;
    }
}
share|improve this question
add comment (requires an account with 50 reputation)

1 Answer

up vote 5 down vote accepted

I wholeheartedly agree that the attributes are evil. But in my opinion having to derive from a base class is just as bad as having to add attributes. Both makes you depend on framework infrastructure you don't need to depend on. (Models should be model logic, and model logic only.)

In MVC (3 at least) you can implement a ModelValidatorProvider and a ModelMetadataProvider. The validator provider can be added in Application_Start using ModelValidatorProviders.Providers.Add(...), and the metdata provider can be injected using the standard IoC container.

The guys who wrote Professional ASP.NET MVC 3 also created fluent implementations of those, so you can have configuration like this:

validationProvider.ForModel<PersonModel>()
    .ForProperty(p => p.Name).Required()
    .ForProperty(p => p.Email).Required().Email();

This will also be picked up by the unobtrusive validation jQuery plugin, and everything else in MVC that would've used the attributes for.

As a sidenote, since you also derive from DynamicObject in your base class, the compiler won't complain if someone inadvertedly attempts to set person.name instead of person.Name.

share|improve this answer
Looking at this again I agree with your thoughts 'having to derive from a base class is just as bad as having to add attributes' Thanks Lars-Erik – eiu165 Jan 26 '12 at 15:35
add comment (requires an account with 50 reputation)

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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