2
\$\begingroup\$

I wrote a method for validating properties based on passed conditions. It works well, but declaration takes more space than the actual condition. Is there any way to make it shorter?

public virtual string Validate(string propertyName, params Tuple<Expression<Func<bool>>, string>[] predicates)
{
    if (!ValidationProperties.ContainsKey(propertyName))
        throw new ArgumentException($"No property found with name '{propertyName}'. Make sure you register it first.");

    foreach (var predicate in predicates)
    {
        if (predicate.Item1.Compile().Invoke())
        {
            ValidationProperties[propertyName] = false;
            ValidationChangedEvent?.Invoke(this, false);
            return predicate.Item2;
        }
    }

    ValidationProperties[propertyName] = true;

    if (ValidationProperties.All(pair => pair.Value))
        ValidationChangedEvent?.Invoke(this, true);

    return null;
}

And here's example of call which I want to be shorter:

 var result = Validate(nameof(FilePath), 
                        new Tuple<Expression<Func<bool>>, string>(() => string.IsNullOrEmpty(FilePath), "Select a file to load"));

Basically I want to get rid of: new Tuple<Expression<Func<bool>>, string>

\$\endgroup\$
0

2 Answers 2

5
\$\begingroup\$

The class encapsulating the two properties suggested by @eurotrash is definitely a good idea. I think name ValidationRule should be ok.

class ValidationRule
{
    public ValidationRule(Func<bool> predicate, string message)
    {
        Predicate = predicate;
        Message = message;
    }
    public Func<bool> Predicate { get; }
    public string Message { get; }
}

I'd go further and create a collection so that I can use the collection initializer to specify the rules. This means collection class must implement the IEnumerable<T> interface and some Add(..) method. Then we can use the syntax:

new SomeCollection
{
   Item1, Item2
}

or if the Add(..) requires more then one parameter:

new SomeCollection
{
   { Item1, Item1a },
   { Item2, Item2a },
}

so our new collection is

class ValidationRuleCollection : IEnumerable<ValidationRule>
{
    private readonly List<ValidationRule> _rules = new List<ValidationRule>();

    public ValidationRuleCollection Add(Func<bool> predicate, string message)
    {
        _rules.Add(new ValidationRule(predicate, message));
        return this;
    }

    public IEnumerator<ValidationRule> GetEnumerator() => _rules.GetEnumerator();

    IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
}

Then you could use it like this

var result = Validate(nameof(FilePath), new ValidationRuleCollection
{
    { () => string.IsNullOrEmpty(FilePath), "Select a file to load" },
});

You don't need the Expression for it. You're not using it here. It adds overhead but does not have any advantage in this case.


The foreach loop can be replaced with LINQ. You stop at the first true result so use the FirstOrDefault to find it.

var rule = validationRules.FirstOrDefault(x => x.Predicate());
if (rule != null)
{
    ValidationProperties[propertyName] = false;
    ValidationChangedEvent?.Invoke(this, false);
    return rule.Message;
}
\$\endgroup\$
0
3
\$\begingroup\$

Basically I want to get rid of: new Tuple<Expression<Func<bool>>, string>

Create your own class instead of the lengthy Tuple?

class X // not sure what to call it
{
    public Expression<Func<bool>> Predicate { get; }
    public string ErrorMessage { get; }

    public X(Expression<Func<bool>> predicate, string errorMessage)
    {
        Predicate = predicate;
        ErrorMessage = errorMessage;
    }
}

static string Validate(string propertyName, params X[] predicates)
{
    // ...        
}

var result = Validate(nameof(FilePath),
    new X(() => string.IsNullOrEmpty(FilePath), "Select a file to load"));
\$\endgroup\$
1
  • \$\begingroup\$ Exactly what I was doing guess you were faster ^^. \$\endgroup\$
    – Denis
    Dec 25, 2016 at 14:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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