Method to validate if string has only hyphens, spaces, Uppercase and digits-CodePudding

Recently I've been trying to create a method to make sure a string has certain types of chars. Yet when I tried using the method, it stops checking if the chars are that way on the 4th character.

For example I tried using 4 lowercase letters and it tells me it's false, but when I try to use invalid characters (lowercase, symbols) on the 5th character, it says it's true:

public bool CarLicensePlateCheck(string m)
{
        if (m.Length >= 4 && m.Length <= 12)
        {
            foreach(char c in m)
            {
                if (char.IsDigit(c) == true ||  char.IsUpper(c)==true || char.IsWhiteSpace(c)==true || c.Equals("-"))
                    return true;
                else
                    break;
            }
            return false;
        }
        else
            return false;
}

CodePudding user response：

You can try regular expressions and let .Net perform check for you:

  using System.Text.RegularExpressions;

  ...

  public static bool CarLicensePlateCheck(string m) =>
    m != null && Regex.IsMatch(m, @"^[A-Z0-9\s\-]{4,12}$");

the pattern is

  ^                  - start of the string
  [A-Z0-9\s\-]{4,12} - from 4 to 12 characters which are either
                         in A..Z range - upper case letters
                         in 0..9 range - digits
                         \s            - white space
                         \-            - dash
  $                  - end of the string

CodePudding user response：

You need to check if the character is not valid and return false immediately and then return true outside of the foreach (because at that point you've validated all the characters). What you have stops validating after the first character because if the first character is valid it just returns true and if it isn't it breaks from the foreach and returns false.

public bool CarLicensePlateCheck(string m)
{
    if (m.Length >= 4 && m.Length <= 12)
    {
        foreach(char c in m)
        {
            // If it's not a valid character return false
            if (!char.IsDigit(c) 
                && !char.IsUpper(c)
                && !char.IsWhiteSpace(c)
                && c != '-')
                return false;
        }

        // all characters have been validated so return true
        return true;
    }
    else
        return false;
}

CodePudding user response：

As the code is written now, it only checks the first character. If this character is valid CarLicensePlateCheck returns true. If it is invalid, the loop is broken and CarLicensePlateCheck returns false.

Check instead that the character is invalid and return false immediately, if so. Return true when the whole loop has passed.

CodePudding user response：

Another option to those presented in the other answers is to use LINQ. This results in code that is very short and I think makes the intent very clear.

using System.Linq;
...
public static bool IsValid(char c)
{
    return char.IsDigit(c) || char.IsUpper(c) || char.IsWhiteSpace(c) || c == '-';
}

public static bool CarLicensePlateCheck(string m)
{
    return m.Length >= 4 && m.Length <= 12 && m.All(c => IsValid(c));
}

This code make use of LINQ's All method, which returns true if a predicate returns true for all elements of a sequence. I also added the static modifier to flag that these methods no not rely on any member variables or properties of the containing class.

Since each function is essentially a single expression this can be rewritten as:

public static bool IsValid(char c) => 
    char.IsDigit(c) || char.IsUpper(c) || char.IsWhiteSpace(c) || c == '-';

public static bool CarLicensePlateCheck(string m) => 
    m.Length >= 4 && m.Length <= 12 && m.All(IsValid);

It's a matter of taste which of these you prefer.