Fdzfgy

I passed a technical test the other day, one part included a cheap string validation and I am wondering this can be further improved to be more versatile to requirement changes.

The requirements were something like that

Create a Validate method, which accepts a string and returns true if it's valid and false if it's not.

A string is valid if it satisfies the rules below:

• The string must be at least 6 characters long and not exceed 16 characters.


• The string must contain only letters, numbers and optionally one hyphen (-).


• The string must start with a letter, and must not end with a hyphen.
  For example, validate("Michelle Belle"); would return false because it contains a space.

My solution was like that:

public static class ComparableExtensions

{

    public static bool IsStrictlyLowerThan<TComparable>(this TComparable comparable, TComparable value)

        where TComparable : IComparable<TComparable>

    {

        return comparable.CompareTo(value) < 0;

    }



    public static bool IsStrictlyGreaterThan<TComparable>(this TComparable comparable, TComparable value)

        where TComparable : IComparable<TComparable>

    {

        return comparable.CompareTo(value) > 0;

    }



    public static bool IsStrictlyNotBetween<TComparable>(this TComparable comparable, TComparable lowerBound, TComparable upperBound)

        where TComparable : IComparable<TComparable>

    {

        if (lowerBound.IsStrictlyGreaterThan(upperBound))

        {

            throw new ArgumentOutOfRangeException(nameof(lowerBound) + nameof(upperBound));                

        }



        return comparable.IsStrictlyLowerThan(lowerBound) || comparable.IsStrictlyGreaterThan(upperBound);

    }

}



public static class CharExtensions

{

    public static bool IsLetterOrDigit(this char c)

    {

        return char.IsLetterOrDigit(c);

    }



    public static bool IsLetter(this char c)

    {

        return char.IsLetter(c);

    }



    public static bool IsHyphen(this char c)

    {

        return c == '-';

    }

}



public class Test

{

    public static bool Validate(string str)

    {

        if (str.Length.IsStrictlyNotBetween(6, 16))

        {

            return false;

        }



        if (!str.First().IsLetter() || str.Last().IsHyphen())

        {

            return false;

        }



        var hyphenCount = 0;



        for (var i = 1; i < str.Length - 1; i++)

        {

            if (str[i].IsLetterOrDigit())

            {

                continue;

            }

            if (str[i].IsHyphen())

            {

                hyphenCount++;

                if (hyphenCount > 1)

                {

                    return false;

                }

            }

            else

            {

                return false;

            }

        }



        return true;

    }

}

I purposefully decided to not go with Regular Expressions to keep the logic readable and I am wondering if my code can be further refactored to incorporate new business rules.

Not much to say about the extensions methods, as they are mostly wrappers.

However if you're looking for ways to make the algorithm more readable, LINQ is your friend. You can replace most of your logic with a one-liner:

var hyphenCount = 0;



for (var i = 1; i < str.Length - 1; i++)

{

    if (str[i].IsLetterOrDigit())

    {

        continue;

    }

    if (str[i].IsHyphen())

    {

        hyphenCount++;

        if (hyphenCount > 1)

        {

            return false;

        }

    }

    else

    {

        return false;

    }

}



return true;

Like this:

return str.All(c => c.IsHyphen() || c.IsLetterOrDigit()) && str.Count(c => c.IsHyphen()) <= 1;

Which more clearly explains your intent, you can also move those expression to their separate methods to make it as readable as possible, this way you can keep adding new conditions, without modifying the existing logic (unless they interfere with one another that is).

• There's a bug: you're not checking if the last character is a letter or digit, only that it isn't a hyphen, so this fails to reject "abcdef&". Denis' solution may be less efficient (2 iterations instead of 1), but with at most 16 characters that's not much of a concern, and it's both easier to read and it works correctly.


• The first two rules read very nicely. I especially like that the business rules are translated to code almost 1:1, that will make updating easier. However, I do think those extension methods are over-engineered. str.Length < 6 || str.Length > 16 and !char.IsLetter(str.First()) || str.Last() == '-' is already quite readable, and that doesn't require extra code that needs to be understood and maintained.


• You can use => syntax for methods with single-expression bodies.

About versatility my thought is a separate class to stipulate the different possible rules:

class ValidationRules

{

    public const char HYPHEN = '-';

    public readonly int hyphenCount;

    public readonly bool needUpper;

    public readonly bool needLower;

    public readonly bool allowSpaces;

    public readonly int minLength;

    public readonly int maxLength;

    /// <summary>

    /// Constructor with min and max length and default rules:

    /// needUpper = true

    /// needLower = true

    /// allowSpaces = false

    /// hyphenCount = 1;

    public ValidationRules(int minLength, int maxLength)

    {

        this.minLength = minLength;

        this.maxLength = maxLength;

        hyphenCount = 1;

        needLower = true;

        needUpper = true;

        allowSpaces = false;

    }

    /// <summary>

    /// Constructor with supplied rules:

    public ValidationRules(int minLength, int maxLength, int hyphenCount, bool needUpper, bool needLower, bool allowSpaces)

    {

        this.minLength = minLength;

        this.maxLength = maxLength;

        this.hyphenCount = hyphenCount;

        this.needLower = needLower;

        this.needUpper = needUpper;

        this.allowSpaces = allowSpaces;

    }

}

the method to validate is still quite simple:

/// <summary>

/// Validate string according to validation rules

/// </summary>

/// <returns></returns>

public static bool Validate(string input,ValidationRules rules)

{

   if(input.Length < rules.minLength || input.Length > rules.maxLength)

    {

        return false;

    }

   if(!Char.IsLetter(input[0]) || input[input.Length-1] == ValidationRules.HYPHEN)

    {

        return false;

    }

    return input.Count(x => x =='-') <= rules.hyphenCount && input.All(x => 

                                                (rules.needUpper && char.IsUpper(x)) || 

                                                (rules.needLower && char.IsLower(x)) || 

                                                (rules.allowSpaces && char.IsWhiteSpace(x)) || 

                                                (x == ValidationRules.HYPHEN);

}