2
\$\begingroup\$

I came up with a validation design for my project and wanted to hear if there's anything I could improve.

Here's how it works. I have the models (code first)

public class Item : IEntity<Item>
{
    public Item()
    {

    }

    public int ID { get; set; }
    public string StoreCode { get; set; }
    public string UPCCode { get; set; }
    public string Description { get; set; }
    public double ABV { get; set; }
    public string URL { get; set; }
    public string Producer { get; set; }

    //each model has its validation method...
    public bool IsValid(Repository<Item> repository)
    {
        if (repository.Single(x => x.StoreCode == this.StoreCode) != null)
            return false;
        else
            return true;
    }
}


public class Price : IEntity<Price>
{       
    public Price()
    {

    }

    public int ID { get; set; }
    public double Value { get; set; }
    public DateTime FetchedDate { get; set; }

    public bool IsValid(Repository<Price> repository)
    {
        var price = repository.Find(x => x.ItemID == this.ItemID).OrderByDescending(x => x.FetchedDate).Take(1).SingleOrDefault();

        if (price != null && price.Value == this.Value)
            return false;
        else if (price == null)
            return true;
        else
        {
            price = repository.Find(x => x.ItemID == this.ItemID && x.FetchedDate.ToString("yyyyMMdd") == this.FetchedDate.ToString("yyyyMMdd")).SingleOrDefault();
            return (price != null) ? false : true;
        }

    }
}

The IEntity interface is as such:

public interface IEntity<TEntity> where TEntity: class, IEntity<TEntity>
{
    int ID { get; set; }
    bool IsValid(Repository<TEntity> repository);

}

And in my repository class, I have an IsValid method, among other read/write methods.

public class Repository<TEntity> where TEntity: class, IEntity<TEntity>
{
    private EntitiesContext = new EntitiesContext ();
    private DbSet<TEntity> AppDbSet;

    public Repository()
    {
        this.AppDbSet = Context.Set<TEntity>();
    }

    public bool IsValid(TEntity entity)
    {
        return entity.IsValid(this);
    }

    //other Get, Single, Update, Add, Save methods...
}

And here's how I use it:

Repository<Price> priceRepo = new Repository<Price>();

//...gather data...

foreach (Price price in prices)
{
    if (priceRepo.IsValid(price))
        priceRepo.Add(price);
    }
}

Is that a good way to implement such a feature? Any areas for improvement (not in the validation methods themselves, but the approach in general)? This will be mostly used for a Web API project.

\$\endgroup\$
  • 1
    \$\begingroup\$ @eurotrash Please do post all suggestions for improvements in answers, even trivial ones. Comments should be for helping to clarify the question, and are subject to deletion. \$\endgroup\$ – 200_success Jan 20 '16 at 21:44
3
\$\begingroup\$

You are close to a standard validation implementation of Entity Framework, which is using IValidatableObject. It may be worth the effort to replace your validation by this readily available validation framework.

The advantage of using IValidatableObject is that it works together with validations applied by data annotations or fluent mappings. If, for example, you have ...

[Required]
public string UPCCode { get; set; }

... or ...

modelBuilder.Entity<Item>().Property(i => i.UPCCode).IsRequired();

... EF will carry out the validation when SaveChanges is executed. If validation fails, a DbEntityValidationException is thrown that contains a property EntityValidationErrors showing the validation errors.

All this happens without even implementing IValidatableObject. If however you also implement this interface, you can add custom validations that blend seamlessly with the ones from data annotations/mappings. Here's how to do that, reducing it to the essentials:

public class Item : IValidatableObject
{
    public int ID { get; set; }
    [Required]
    public string StoreCode { get; set; }

    public IEnumerable<ValidationResult> Validate(ValidationContext validationContext)
    {
        var context = (DbContext)validationContext.Items["Context"];
        var item = (Item)validationContext.ObjectInstance;
        if (context.Set<Item>().Any(c => c.StoreCode == item.StoreCode && c.ID != item.ID))
        {
            var error = string.Format("StoreCode '{0}' exists", item.StoreCode);
            yield return new ValidationResult(error, new[] { "StoreCode" });
        }
        yield return ValidationResult.Success;
    }
}

Now if this validation is violated, it will also throw a DbEntityValidationException with the custom message in EntityValidationErrors.

Notice that the context responsible for saving the item is wrapped in the validationContext. This is not so by default. You must override the context's DbEntityValidationResult method to add the context to validationContext.Items:

protected override DbEntityValidationResult ValidateEntity(DbEntityEntry entityEntry, IDictionary<object, object> items)
{
    items.Add("Context", this);
    return base.ValidateEntity(entityEntry, items);
}

You may have noticed another big advantage, compared to your approach: the Validate method returns meaningful information rather than just a boolean. I'm not even sure how you would supply feedback from your Validate method.

As you see, this is close enough to consider refactoring your own architecture, but also different enough to make this more than a trivial operation. It's more at the ground level, while your validation is in an abstraction layer.

Another difference is that the EF validation is holistic: it validates an entire object graph, while your validation validates individual entities. Both have their pros and cons. With EF's validation it's harder to get validation errors for one specific entity, with your validation it's hard to validate a mixed collection of objects.

\$\endgroup\$

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.