3

I can't really think of a better way to word my title and it's probably the most accurate description of what I've just done. This question is a mix of best practices and coding style.

I've basically got a WPF application that uses the MVVMLight libraries and Entity Framework (6.x) with the database first workflow.

One of the POCO classes generated from the database has about 44 columns, just keep that in mind for a moment.

For validating this particular object. I'm extending the functionality of the POCO object using a partial class which implements IDataErrorInfo like so:

public partial class MyClass : IDataErrorInfo { ... }

Now this is ok, until you remember this object has 44 fields. This is going to give me an indexer that's going to be 180+ lines of if-statements.

This to me just screams bad programming practice. The structure of this indexer looks like this:

public string this[string columnName]
{
    get
    {
        switch (columnName)
        {
            case "Column1":
                if (string.IsNullOrWhiteSpace(Column1))
                    return "A value for Column1 is required.";
                // More if-statements here...
                break;

            case "Column2":
                // Same as above.
                break;

            // There's going to be about another 42 cases here...
        }

        return null;
    }
}

Other things that I have considered is breaking the if statements into separate methods, which would reduce the amount of lines in the indexer, but introduce 40+ methods which all have the same structure.

// In the indexer.
switch(columnName)
{
    case "Column1":
        return ValidateColumn1();
        break;
    case "Column2":
        return ValidateColumn2();
        break;
}

// Somewhere a bit further down the class...
private string ValidateColumn1()
{
    if (string.IsNullOrWhiteSpace(Column1))
       return "A value for Column1 is required.";
    // More if-statements...
}

private string ValidateColumn2()
{
    // Ditto.
}

I can appreciate that there's a number of ways that someone can do validation in WPF/EF such as:

but I'm just curious as to what the best way to approach this is since a 200+ line indexer and creating 40+ methods/classes just seems a really wrong way to go about things.

I understand that I'm probably asking multiple questions here, but this is what I'm trying to understand:

  1. Using the first code snippet in this question as the example, is it considered good practice to validate each column in the POCO object in the indexer using if-statements? For an object that has 5 columns, this seems ok, but with 44 it becomes unwieldy and a maintenance nightmare. (Even though this could potentially be a normalisation issue in the database.)
  2. What is the best/preferred way to implement the validation or does it depend on the programmers requirements?
  3. Is there any other ways that I could address validating my POCO objects? I've looked at the FluentValidation library, but this is doing more harm than good because I seem to be throwing all these different libraries together and hoping they'll play nice together, which they never seem to do.

Thanks in advance.

Community
  • 1
  • 1
Jake
  • 1,701
  • 3
  • 23
  • 44
  • For IDataErrorInfo, how about using a Dictionary to store errors for specific properties? At least that seems to be the preferred way for the new INotifyDataErrorInfo interface: https://msdn.microsoft.com/en-US/library/windows/apps/system.componentmodel.inotifydataerrorinfo(v=vs.105).aspx – vesan Jun 09 '15 at 05:13
  • @vesan, I've looked at that link and can I just clarify, would I need to explicitly write the get/set accessors for the properties in my POCO objects to use that interface as the T4 template generates each property like this: `public datatype PropertyName` Or rather, would I need to change the template itself in some way? – Jake Jun 09 '15 at 05:27
  • I'm not sure what your generated code looks like, but yes, the idea is that validation is run every time a property changes, which would require a custom set accessor. – vesan Jun 09 '15 at 05:38
  • @vesan, Would changing the template to write the properties as virtual be suitable? i.e. `public virtual string MyProperty;` – Jake Jun 09 '15 at 05:44
  • Update: Just tried adding virtual to the template, it did nothing. Claimed there was an ambiguity. – Jake Jun 09 '15 at 05:52
  • I don't think so. `public virtual string MyProperty;` is actually not a property - it is a field. – vesan Jun 09 '15 at 06:03
  • One suggestion independent from the approach. I think it's bad to have the indexer of the entity that does validation so I think it could be better use explicit IDataErrorInfo implementation. – bubi Jun 09 '15 at 06:53

1 Answers1

1

This is an approach (I usually do this). I use EF data annotation (in my case I map every entity with EF data annotation and with EF fluent interface for relations). I usually inherit from an EntityBase with standard IDataErrorInfo. This is a piece of my EntityBase

public class EntityBase : IDataErrorInfo
{


    public virtual bool IsValid()
    {
        return GetValidationErrors() == string.Empty;
    }

    protected virtual string GetValidationErrors()
    {
        var vc = new ValidationContext(this, null, null);
        var vResults = new List<ValidationResult>();

        if (!Validator.TryValidateObject(this, vc, vResults, true))
            return vResults.Aggregate("", (current, ve) => current + (ve.ErrorMessage + Environment.NewLine));

        return "";
    }

    protected virtual string GetValidationErrors(string columnName)
    {
        var vc = new ValidationContext(this, null, null);
        var vResults = new List<ValidationResult>();
        if (!Validator.TryValidateObject(this, vc, vResults, true))
        {
            string error = "";
            foreach (var ve in vResults)
            {
                if (ve.MemberNames.Contains(columnName, StringComparer.CurrentCultureIgnoreCase))
                    error += ve.ErrorMessage + Environment.NewLine;

            }
            return error;
        }

        return "";
    }

    string IDataErrorInfo.Error
    {
        get { return GetValidationErrors(); }
    }

    string IDataErrorInfo.this[string columnName]
    {
        get { return GetValidationErrors(columnName); }
    }

}

There are entities that needs a complex property validation (i.e. cross property validation). In this case I override the virtual methods and add specific validation to the entity

bubi
  • 6,414
  • 3
  • 28
  • 45