CODEWITHSUNDEEP
c#
Martijn
Martijn

Reputation: 24789

C# A validate method which can return multiple messages

I have a situation where I need to check a couple of things before I proceed with saving into the database.

If I want to save an object to the database I have to check two things:

  1. Are all the required things filled?
  2. Is there already a thing with the same status?

If one of those requirements fails, I'd like to notify the user that it cannot be saved because condition 1 is not met, or condition 2 is not or both conditions are not met.

Should I just create a method named Validate() and put the textual message into the return parameter? Something like

public string Validate()
{
  string message = string.Empty;

  if(! /*Do some validating for 1) */)
    message = "Condition 1 not met";
  if(! /*Do some validating for 2) */)
    message += Environment.NewLine + "Condition 2 not met!"; // 

  return message;
}

And then in the calling code check if message is empty or not. Is this a good way?

I have also been thinking about two methods. But then I have to call these two methods everywhere where I want to save the object to the database. This way I am repeating myself. This way you get something like this:

public bool AreRequiredThingsFilled()
{
  if( /* do something*/ )
    return true;

  return false;
}

public bool CheckStatus()
{
  if( /* do something*/ )
    return true;

  return false;
}

And then I have to call both methods and in the calling code I have to set the message which I want to show to the user.

I don't know which way to do handle this kind of situations. Is there a best practice for such kind of situations?

Upvotes: 1

Views: 4446

Answers (8)

Wayne Feltham
Wayne Feltham

Reputation: 541

Here's how I'd do this...

  1. I have a generic return type

    public class ValidatorResult
{
    public ValidatorResult()
    {
        this.Failiures = new List<string>();
    }

    public bool IsValid => !this.Failiures.Any();

    public List<string> Failiures { get; set; }
}

  2. I create an interface that defines my checks...

    public interface ISalesOrderValidator
{
    ValidatorResult ValidateParams(CreateSalesOrderParams obj);

    ValidatorResult ValidateParams(UpdateSalesOrderParams obj);

    ValidatorResult ValidateParams(ApplyDiscountParams obj);
}

  3. ...and a Base Class...

    public class BaseValidator
{
    public BaseValidator()
    {
        this.ValidatorResult = new ValidatorResult();
    }

    public ValidatorResult ValidatorResult { get; set; }
}

  4. I then implement my interface...

    public class SalesOrderValidator : BaseValidator, ISalesOrderValidator
{
    ValidatorResult ValidateParams(CreateSalesOrderParams obj)
    {
        try
        {
            if (obj.Payee == null)
                ValidatorResult.Failiures.Add("You must proivde a Payee");

            if (obj.PaymentAmount == 0)
                ValidatorResult.Failiures.Add("PaymentAmount is 0");                
        }
        catch (Exception)
        {
            ValidatorResult.Failiures.Add("An unexpected error occurred.");
        }

        return base.ValidatorResult;
    }

    ValidatorResult ValidateParams(UpdateSalesOrderParams obj)
    {
        throw new NotImplementedException();
    }

    ValidatorResult ValidateParams(ApplyDiscountParams obj);
    {
        throw new NotImplementedException();
    }
}

  5. Then, within my code where I need to do the check, I call my validator...

     // start by doing some basic validation on the parameters
 var validationResult = _validator.ValidateParams(mySalesOrder);
 if (!validationResult.IsValid)
     throw new ValidationException(validationResult.Failiures.FirstOrDefault());

...This just returns the first error - you could change the code to return multiple error messages, or an error stack (Inner Exceptions). Note - ValidationException is defined as follows...

    public class ValidationException : ApplicationException
    {
        public ValidationException()
        {
        }

        public ValidationException(string message)
            : base(message)
        {
        }

        public ValidationException(string message, Exception inner)
            : base(message, inner)
        {
        }
    }

Upvotes: 0

benPearce
benPearce

Reputation: 38333

Implement IDataErrorInfo on your object to be validated, the interface allows for error messages to be collected keyed by Property Name.

It also has a string property called Error which can represent a single error for the entire object.

Upvotes: 0

Alex McBride
Alex McBride

Reputation: 7011

An interesting pattern I've seen used before is to return an IEnumerable of messages.

public bool HasValidationErrors()
{
    return GetValidationErrors().Any();
}

public IEnumerable<string> GetValidationErrors()
{
    if (/* Some Condition */)
    {
        yield return "condition 1 not met";
    }

    if (/* Some Condition */)
    {
        yield return "condition 2 not met";
    }

    yield break;
}

Then in your code you would do:

if (HasValidationErrors())
{
    foreach (string error in GetValidationErrors())
    {
        // Do something with 'error'
    }
}

You could return an error object instead of a string of course.

Upvotes: 4

Scott DePouw
Scott DePouw

Reputation: 3899

In a previous project, we had a ValidationResult class.

public class ValidationResult
{
    public bool Success;
    public string Message;
}

This provided a simple mechanic for returning both whether or not validation succeeded and a corresponding failure message if one occurred. If our results required more information, we simply inherited from ValidationResult and extended the class that way.

Upvotes: 1

jgauffin
jgauffin

Reputation: 101150

I would use an existing validation framework instead of doing it myself.

Upvotes: 0

CodesInChaos
CodesInChaos

Reputation: 108810

I'd either define a custom return type (Message collection) which is the clean solution.

Or as a quick hackish solution return a bool and output the additional info using an out param.

bool Validate(out string[] messages)
{
    ...
}

bool Validate()
{
    string[] messages;
    return Validate(messages);
}

Upvotes: 0

Patrick Peters
Patrick Peters

Reputation: 9568

The first implementation is a maintenance nightmare. All "client" code depends on the way you concatinate the stuff.

The second is also not preferred; you'll get a ton of public methods to validate in the future.

A good practice is to get the domain logic as a POCO style: create an object that holds the state of the information you will need. Let the object derive from a base class with a general validate method. Let each specific object override the validate method and do some specific validation stuff related to the properties of that object. Let each validate method pump their error messages in a list of strings, for example a List or IQueryable.

Upvotes: -1

Jagmag
Jagmag

Reputation: 10356

In case of a generic method like Validate, i would prefer to return a collection.

Usually, if we are talking about Business object, there will be multiple validations on various aspects of its data that will need to be performed. Hence, the validate should return a collection with a list of all the errors encountered.

The caller can always loop through the collection and then decide what to do with it.

Though you can return a list<string>, depending on your needs, you could consider having a custom object collection to return eg MessageCollection . This is because sometimes, there is additional data to be sent back eg: different types of error categories / warnings v/s errors etc, Either OR conditions etc.

Also, depending on whether internationalization of the error messages is a requirement, it would be better to maybe return just an error code from the actual business object and let the UI map it to the correct error message in the correct language based on the user's settings.

Upvotes: 1

Related Questions