Validation invoking queries? I have a question regarding whether it is viable or deemed correct if a validation object invoked a query.  Essentially, I have some code that looks like this:

public interface ICountryCodeValidator
{
    IEnumberable&lt;ValidationError&gt; Validate(string countryCode);
}

public class CountryCodeValidator : ICountryCodeValidator
{
    private readonly IGetCountryCodeQuery getCountryCodeQuery;

    public CountryCodeValidator(IGetCountryCodeQuery getCountryCodeQuery)
    {
        this.getCountryCodeQuery = getCountryCodeQuery;
    }

public IEnumberable&lt;ValidationError&gt; Validate(string countryCode)
{
    var errors = new List&lt;ValidationError&gt;();

    if (string.IsNullOrEmpty(countryCode) || countryCode.Length &gt; 2)
    {
        errors.Add(new ValidationError { Code = "InvalidRequest", Message = "Country code is invalid." });
    }
    else
    {
        if (this.getCountryCodeQuery(countryCode) == null)
        {
            errors.Add(new ValidationError { Code = "NotFound", Message = "CountryCode not found." });
        }       
    }

    return errors;
}
}


Immediately, I am thinking that CountryCodeValidator responsibilities are growing and whether it should be querying the DB is debatable. Is it perfectly OK for an object simply responsible for validation to perform such an operation?

Further elaborating on this, I have unit tests surrounding CountryCodeValidator and assert against the error codes returned ensuring the correct codes are returned depending on the input (which is fine). 

The trouble is, within my Web API controller I have to perform checks on the error code being returned back and depending on the error code create the appropriate status code. For example:

public class CountriesContoller : ApiController
{
    private readonly ICountryCodeValidator countryCodeValidator;
    private readonly ICountryCodeRepository countryCodeRepository;

    public CountriesContoller(ICountryCodeValidator CountryCodeValidator, ICountryCodeRepository CountryCodeRepository)
    {
        this.countryCodeValidator = countryCodeValidator;
        this.countryCodeRepository = countryCodeRepository;
    }

    public HttpResponseMessage Get([FromUri]string countryCode)
    {
        var errors = this.countryCodeValidator.Validate(countryCode);

        if (errors.Any())
        {
            //Code smell?
            var countryNotFoundError = errors.FirstOrDefault(x =&gt; x.Code == "NotFound");

            if (countryNotFoundError != null)
            {
                return Request.CreateResponse(HttpStatusCode.NotFound, countryNotFoundError);
            }

            return Request.CreateResponse(HttpStatusCode.BadRequest, errors);
        }

        return Request.CreateResponse(HttpStatusCode.OK, this.countryCodeRepository(countryCode));
    }
}


I guess, the bit I am concerned with is the fact I am leaking the error code definitions in each of my domains and tests.


Web API layer I am performing a linq query to check if this error has been added
Domain layer needs to know about this due to validation
Tests are aware as I am testing whether a NotFound is invoked, so when I mock the CountryCodeValidator I am ensuring it returns a collection of errors with a particular error code.


A quick easy thing to do is create constants or an enum so I am not using magic strings but still this doesn't seem right.

I realise I have 2 calls to the database being made:


One to verify the call is not null
Second to get the country and return it back


This is simply pseudo code.
I believe it's ok to use db data inside a validator's body. As long as all you do with database is reading data, and this data is neccesary for validation. But if you're about to change something in database - that's pretty considerable.

Then, if you can do something with this data, like caching it or smth, you may want to consider moving this part of validator's logic into some abstraction, like CountryCodeHandler or something like this.

I would also point out on your working with string values. I don't think it's good to use those "Not found" or "InvalidRequest" strings. If the string is exactly what you need, i'd advise you to use some static class:

public static class MyHttpStatusCode
{
    public const string NotFound = "Not Found";
    public const string InvalidRequest = "Invalid Request";
}


Thus, you'll get rid of duplicates in your code. Or, if those strings aren't that unavoidable, you can just use HttpStatusCode enum from System.Web (as i recall).

Also in modern versions of C# you don't need the "this" keyword to access to fields and properties those names arent match any variable inside the method or constructor. Unless it's your codestyle that declares such thing.

As for your controller part, i'd recommend you to use the Chain of Responsibility pattern. It will allow you to extend validation results and handle them properly.

public interface ICountryCodeValidationResultFilter
{
    ValidationError[] Filter(List&lt;ValidationError&gt; errors, out HttpStatusCode statusCode);
}
public interface IConcreteCodeValidationResultFilter
{
    bool CanFilter(List&lt;ValidationError&gt; errors);
    ValidationError[] Filter(List&lt;ValidationError&gt; errors, out HttpStatusCode statusCode);
}

public class CountryCodeValidationResultFilter : ICountryCodeValidationResultFilter
{
    private readonly IConcreteCodeValidationResultFilter[] filters;

    public CountryCodeValidationResultFilter(IConcreteCodeValidationResultFilter[] filters)
    {
        this.filters = filters;
    }

    public ValidationError[] Filter(List&lt;ValidationError&gt; errors, out HttpStatusCode statusCode) 
    {
        if (errors.Any())
        {
            for (var filter in this.filters) // you may just write "filters" here
            {
                if (filter.CanFilter(errors))
                {
                    return filter.Filter(errors, out statusCode);
                }
            }
        }

        statusCode = HttpStatusCode.OK;
        return new ValidationError[0];
    }
}

public NotFoundCodeValidationResultFilter : IConcreteCodeValidationResultFilter
{
    public bool CanFilter(List&lt;ValidationError&gt; errors)
    {
        return errors.Any(e =&gt; e.Code == "Not Found");
    }

    public ValidationError[] Filter(List&lt;ValidationError&gt; errors, out HttpStatusCode statusCode)
    {
        statusCode = HttpStatusCode.NotFound;
        return new[] { errors.First(e =&gt; e.Code == "Not Found") };
    }
}

public InvalidRequestCodeValidationResultFilter : IConcreteCodeValidationResultFilter
{
    public bool CanFilter(List&lt;ValidationError&gt; errors)
    {
        return errors.All(e =&gt; e.Code != "Not Found");
    }

    public ValidationError[] Filter(List&lt;ValidationError&gt; errors, out HttpStatusCode statusCode)
    {
        statusCode = HttpStatusCode.NotFound;
        return errors.ToArray();
    }
}


Thus, your controller will look like this:

public class CountriesContoller : ApiController
{
    private readonly ICountryCodeValidator countryCodeValidator;
    private readonly ICountryCodeRepository countryCodeRepository;
    private readonly ICountryCodeValidationResultFilter countryCodeValidationResultFilter;

    public CountriesContoller(
        ICountryCodeValidator countryCodeValidator,
        ICountryCodeRepository countryCodeRepository,
        ICountryCodeValidationResultFilter countryCodeValidationResultFilter)
    {
        this.countryCodeValidator = countryCodeValidator;
        this.countryCodeRepository = countryCodeRepository;
        this.countryCodeValidationResultFilter = countryCodeValidationResultFilter;
    }

    public HttpResponseMessage Get([FromUri]string countryCode)
    {
        var errors = countryCodeValidator.Validate(countryCode);
        HttpStatusCode resultStatusCode;
        var filteredErrors = countryCodeValidationResultFilter.Filter(errors, out resultStatusCode);

        return Request.CreateResponse(resultStatusCode, filteredErrors.Any() ? filteredErrors : countryCodeRepository(countryCode));
        // i believe countryCodeRepository is not just Func, huh? perhabs it should be "countryCodeRepository.Get(countryCode)" ?
    }
}