Validating Firstname and Lastname

Question

I have written some code in Java that checks if the strings Firstname and Lastname are valid names. These are inputted by the user. My code works as expected, but I would like to optimize and shorten the code such that it is readable and understandable.

Both these strings, Firstname and Lastname, will not be null or empty and might contain more than one word.

Firstname and Lastname are valid if

1. The length of both Firstname and Lastname are more than two.
2. There isn't any character which isn't an alphabet or space in them.
3. There isn't more than three vowels in a word in Firstname or Lastname, and there isn't more than two consecutive vowels in a word.
4. There isn't more than four consecutive consonants in a word in Firstname or Lastname, and there isn't more than two consecutive same consonant in a word.

The below code works as expected:

/*Returns false if both names are valid and true if they aren't*/
public static boolean errcheck(String Firstname,String Lastname,int security)
 {
    while(true)
    {
      if(security>=2 && !checklen(Firstname,Lastname))break;
      if(security>=3 && !checknum(Firstname,Lastname))break;
      if(security>=4 && !checkvowel(Firstname,Lastname))break;
      if(security>=5 && !checkcon(Firstname,Lastname))break;
      return true;
    }
  return false;
 }

/*Checks if length of both strings are less than three*/
public static boolean checklen(String Firstname,String Lastname)
 {
  if(Firstname.length()<3 || Lastname.length()<3)
     return false;
  return true;
 }

/*Checks if both strings contain non-alphabetic characters excluding a space character*/
public static boolean checknum(String Firstname,String Lastname)
 {
    int length=0,length2=0,i=0;
    char c;
    String Fullname;

            length=Firstname.length();
            length2=Lastname.length();


                for(i=0;i<length;i++)  //Check for `Firstname`
                {  
                    c = Firstname.charAt(i);  
                    if(!((c>='a' && c<='z')||(c>='A' && c<='Z')) && c!=' ')
                    {
                         return false;
                    }
                } 

                for(i=0;i<length2;i++)    //Check for `Lastname`
                {  
                    c = Lastname.charAt(i);  
                    if(!((c>='a' && c<='z')||(c>='A' && c<='Z')) && c!=' ')
                    {
                        return false;
                    }
                }



 return true;
 }

/*Checks if both strings contain more than 2 consecutive vowels. Also checks if they contain more than 3 vowels. Both are done in each word*/
public static boolean checkvowel(String Firstname,String Lastname)
 {
    int numv=0,conv=0,length=Firstname.length(),length2=Lastname.length();
    char c;
    int i;
    char[] vowel={'a','e','i','o','u','A','E','I','O','U','\0'};
    for(i=0;i<length;i++){
        c=Firstname.charAt(i);
        for(char test:vowel)
        {
            if(c!=' ')
            {

                if(test==c) //If current char is a vowel
                {
                    numv++; 
                    conv++; //Increase counters
                    if(conv>2 || numv>3) //Invalid name detected
                        return false;
                    break;
                }
                if(test=='\0')conv=0;
            }else{numv=conv=0;break;} //New word. So reset counters
        }
    }
    numv=conv=0;
    for(i=0;i<length2;i++){ //Do the same for `Lastname`
        c=Lastname.charAt(i);
        for(char test:vowel)
        {
            if(c!=' ')
            {
                if(test==c)
                {
                    numv++;
                    conv++;
                    if(conv>2 || numv>3)
                        return false;
                    break;
                }
                if(test!='\0')conv=0;
            }else{numv=conv=0;break;}
        }
    }
    return true;
 }

/*Checks if both strings contain more than 2 consecutive same consonants. Also checks if they contain more than 4 consonants. Both are done in each word*/
public static boolean checkcon(String Firstname,String Lastname)
 {
    int num=0,length=Firstname.length(),length2=Lastname.length(),con=0;
    char c,tmp='a';
    int i;
    for(i=0;i<length;i++)
    {
        if((c=Firstname.charAt(i))!='a' && c!='e' && c !='i' && c!='o' && c!='u' && c!='A' && c!='E' && c !='I' && c!='O' && c!='U' && c!=' ') //If current character is not a vowel or a space
        {
            num++;
            if(tmp!='a' && c==tmp)
                con++;
            if(num>3 || con>1) //Invalid name
                return false;
            tmp=c;
            continue;
        }
        num=0;
        con=0;
        tmp='a'; 
    }
    num=0;
    con=0;
    tmp='a'; //Reset everything
    for(i=0;i<length2;i++) //Do the same check for `Lastname`
    {
        if((c=Lastname.charAt(i))!='a' && c!='e' && c !='i' && c!='o' && c!='u' && c!='A' && c!='E' && c !='I' && c!='O' && c!='U' && c!=' ')
        {
            num++;
            if(tmp!='a' && c==tmp)
                con++;
            if(num>3 || con>1)
                return false;
            tmp=c;
            continue;
        }
        num=0;
        con=0;
        tmp='a';
    }
 return true;
 }

I would like to shorten and optimize the above code.

Answer 1

Usability

As I mentioned in my comment, you just exclude way too many names (even for a test program; I would at least add a comment in the code). Personally, I would accept any input, do a length check (it should not be empty, and it should fit into the database), and that's it (for further input validation - if it's needed - I would use a web application firewall, or something similar to filter all input). It will be really frustrating for users if they cannot enter their name, and thus have to make up a name.

Security

I don't see the use of your security integer. It's hard to see what it's meaning is from the outside (if you really want this, use an enum), and it's not really needed.

• is a short username really a security risk? No.
• are too many vowels or too many consonants a security risk? Again, not really.

There are three main things to worry about with names (that I can think of):

• SQL injection
• XSS
• repeated, large input that takes up a lot of space

SQL injection is a solved problem (prepared statements), so you don't need to worry about it. XSS is more complicated, but there are pretty good guidelines out there of how to prevent it, and most of your restrictions don't help here at all. That leaves the last point, which you implicitly cover with your vowels/consonants rules. A simple length check would be a lot easier to manage, and be more user friendly.

Misc

• you have quite a bit of duplication in your code (eg when checking for characters in the names). Whenever you copy-paste something, think about extracting it to a function instead.
• your indentation is off (it's not what is commonly used in Java, and it's not even internally consistent). Use any IDE to fix this.
• use more spaces to increase readability.
• declare only one variable per line to increase readability.
• use curly brackets for one-line statements.
• variable names should start with a lowercase character.
• you never use Fullname.
• don't have more than one command on one line. Eg }else{numv=conv=0;break;} is hard to read.
• why is \0 a vowel?
• method names should be written in camelCase (eg errcheck -> errCheck), and I would not abbreviate names (so I would use errorCheck, or just isValidName).
• I would also not use abbreviation for variable names, it makes the code harder to read. Write num and con, numv, etc out, and rename c and tmp.
• declare variables in as small a scope as possible. This also goes for loop variables (eg for(i=0;i<length;i++) -> for(int i = 0; i < length; i++))
• your while-if-break structure is a bit hard to read, I would think about alternatives (eg get rid of the loop, change the breaks to returns, and turn the order of the statements around).

Answer 2

Requirements
The first thing that springs to mind is that the requirements seem a bit limited, are they intended to be used in a real world system or is this a programming exercise

• Ryan O'Neal wouldn't pass the second check, nor would Farrah Fawcett-Majors
• Abigail is not an uncommon name (depends on the location, I suppose) and it would fail the third check
• How would Lon Chaney Jr. or Trey Parker (Randolph Parker III) be handled by the system? Are the suffices considered part of the last name? If so, they fail checks 2 and 3 respectively.

Specific Points
errCheck()
- It really should be called something like validateName.
- Why is the security parameter here? I can see what it does but it isn't in the stated requirements.
Is it really valid to say that sometimes we only care about the length, sometimes about the length and valid chars, sometimes about the length, vaild chars and vowel counts. It seems overkill/confusing to allow that level of customization.
How does the rest of the system deal with names that sometimes cannot have '-' chars in them [we tested them with security level 3] but sometimes can have them [we tested these will level 2]

checkNum()
Like errcheck, it doesn't match to what the function does. Also, the standard is for camel casing, so it should be called checkNum (if it checked the number of anything. checkValidChars ?

We should probably have separate tests for the valid first name and last name characters. It aids reusability - if we want to check the validity of a first name on its own we do not need to have a dummy last name. This may seem like a very picky point but why add a dependency between checking the first and last name when there is none.

Design
I can't say how it would work for performance but in terms of readability and extensibility, I would approach it as a set of tests that can be executed against the first, last or both names.

We have a NameValidator that takes a ITestProvider in the ctor. This allows us to add new tests without having to open up the NameValidator again. Enables easier use of dependency injection.

Each test has a level, a set of TestTypes (we can apply the same test to both first and last names) and an execute method. In this version we just get a simple pass/fail, in a production system I would return a TestResult with details of why it failed.

We can invoke validation on first and last names separately. The validator executes the appropriate tests by level on the input name (assumes some tests can be on both first and last name or else we might have separate lists of firstname and lastname tests)

I am a big fan of regular expressions for this sort or thing. Length and valid chars can be built into a single check e.g. [a-zA-Z]{3}, with separate tests for first and last name if they can have different characters in each.

Apologies on the code, it is generated from C#

public enum TestType
{
    None,
    FirstName,
    LastName
}

public interface ITest
{
    Iterable<TestType> getAppliesTo();
    int getSecurityLevel();
    boolean execute(String name);
}

public interface ITestProvider
{
    Iterable<ITest> getAllTests();
}

public interface INameValidator
{
    boolean isValidFirstName(String name, int level);
    boolean isValidLastName(String name, int level);
}

public class NameValidator implements INameValidator
{
    private Iterable<ITest> _tests;

    public NameValidator(ITestProvider testProvider)
    {
        _tests = testProvider.GetAllTests().ToList();
    }

    public final boolean isValidFirstName(String name, int level)
    {
        return checkName(TestType.FirstName, name, level);
    }

    public final boolean isValidLastName(String name, int level)
    {
        return checkName(TestType.LastName, name, level);
    }

    private boolean checkName(TestType type, String input, int level)
    {
        // in C# I use all the tests of the correct type and security level and execute them on the input. If all pass, then we return true, it any fail, then return a false. Don't know enough java to quickly convert.

    }
}

public class SimpleTestProvider implements ITestProvider
{
    public final Iterable<ITest> getAllTests()
    {
        return new java.util.ArrayList<ITest>(java.util.Arrays.asList(new ITest[] {new FirstNameLengthAndChars()}));
    }
}


public class FirstNameLengthAndChars implements ITest
{

    // C# regex. don't know java equivalent
    private final static Regex Regex = new Regex("[a-z]{3,}", RegexOptions.Compiled | RegexOptions.IgnoreCase);


    public final Iterable<TestType> getAppliesTo()
    {
        return new java.util.ArrayList<TestType>(java.util.Arrays.asList(new TestType[] {TestType.FirstName}));
    }

    public final int getSecurityLevel()
    {
        return 2;
    }

    public final boolean execute(String input)
    {
        return Regex.IsMatch(input);
    }
}

Answer 3

Check out Pattern and Matcher classes from java.util.regex. For example to check that string contains only alpha-numeric characters and spaces:

Pattern p=Pattern.compile("^\w*$");
boolean isMatchFound=p.matcher("Test string").matches();

Answer 4

public static boolean errcheck(String Firstname,String Lastname,int security)
 {
    while(true)
    {
      if(security>=2 && !checklen(Firstname,Lastname))break;
      if(security>=3 && !checknum(Firstname,Lastname))break;
      if(security>=4 && !checkvowel(Firstname,Lastname))break;
      if(security>=5 && !checkcon(Firstname,Lastname))break;
      return true;
    }
  return false;
 }

First, please use spaces around operators and after commas.

Second, I would combine those ifs :

if(security >=2 && !checklen(Firstname, Lastname) ||
   security >=3 && !checknum(Firstname, Lastname) ||
   security >=4 && !checkvowel(Firstname, Lastname) ||
   security >=5 && !checkcon(Firstname, Lastname)) {

    break;
}

Third, it is good practice to use braces around one line statements to help prevent errors.

Fourth, you should keep your indentation at the same level:

public static boolean errcheck(String Firstname,String Lastname,int security)
{
    while(true)
    {
        if(security>=2 && !checklen(Firstname,Lastname))break;
        if(security>=3 && !checknum(Firstname,Lastname))break;
        if(security>=4 && !checkvowel(Firstname,Lastname))break;
        if(security>=5 && !checkcon(Firstname,Lastname))break;
        return true;
    }
return false;
}

This is begging to be rewritten:

int numv=0,conv=0,length=Firstname.length(),length2=Lastname.length();
char c;
int i;
char[] vowel={'a','e','i','o','u','A','E','I','O','U','\0'};
for(i=0;i<length;i++){
    c=Firstname.charAt(i);
    for(char test:vowel)
    {
        if(c!=' ')
        {

            if(test==c) //If current char is a vowel
            {
                numv++; 
                conv++; //Increase counters
                if(conv>2 || numv>3) //Invalid name detected
                    return false;
                break;
            }
            if(test=='\0')conv=0;
        }else{numv=conv=0;break;} //New word. So reset counters
    }
}

This is how I would write this:

int numVowel = 0, conVowel = 0;
char[] vowels = { 'a', 'e', 'i', 'o', 'u', 'A', 'E', 'I', 'O', 'U' };

for(int i = 0; i < Firstname.length(); i++){
    char c = Firstname.charAt(i);
    for(char vowel : vowels)
    {
        if (c == ' ')
        {
            numVowel = 0;
            conVowel = 0;
            break;
        }
        if (vowel == c)
        {
            numVowel++; 
            conVowel++;
            if(conVowel > 2 || numVowel > 3) // Invalid name detected
            {
                return false;
            }
            break;
        }
    }

    if (c == 'U') // Last vowel and character is not a vowel because we got here
    {
        conVowel = 0;
    }
}

Notice how I removed superfluous variables, moved some variables into a tighter scope, used more meaningful variable names, and tidied the code up a bit.