2

The following code should test 2 strings (username and password) for length and allowed characters (should be 0-9. a-z, A-Z). the legalCharacters ArrayList is filled with characters a-z and 0-9.

testing for appropriate length works, but not for legal characters this test returns an illegal characters error in the username: user: aba pass: aba

public DataResponseType checkForDataErrors(String username, String password) {
    responseType = null;
    boolean isIllegalUsername = false;
    boolean isIllegalPassword = false;
    if (username.length() < 3 || username.length() > 15) {
        responseType = DataResponseType.INVALID_USERNAME_LENGTH;
    } else if (password.length() < 3 || password.length() > 15) {
        responseType = DataResponseType.INVALID_PASSWORD_LENGTH;
    } else {
        for (int x = 0; x < username.length(); x++) {
            for (int z = 0; z < legalCharacters.size(); z++) {
                Character test = username.toLowerCase().charAt(x);
                if (!test.equals(legalCharacters.get(z))) {
                    if (z == legalCharacters.size() - 1) {
                        isIllegalUsername = true;
                        break;
                    }
                }
            }
            if (isIllegalUsername) {
                break;
            }
        }
        for (int x = 0; x < password.length(); x++) {
            for (int z = 0; z < legalCharacters.size(); z++) {
                Character test = password.toLowerCase().charAt(x);
                if (!test.equals(legalCharacters.get(z))) {
                    if (z == legalCharacters.size() - 1) {
                        isIllegalPassword = true;
                        break;
                    }
                }
            }
            if (isIllegalPassword) {
                break;
            }
        }
        if (isIllegalUsername) {
            responseType = DataResponseType.INVALID_USERNAME_CHARACTERS;
        } else if (isIllegalPassword) {
            responseType = DataResponseType.INVALID_PASSWORD_CHARACTERS;
        } else {
            responseType = DataResponseType.VALID;
        }
    }
    return responseType;
}

Does anybody know how to fix this? or do it in a easier way (I don't understand how predicates work and haven't been able to find a good explanation)? If other code is needed, please ask.

Jjampong
  • 584
  • 1
  • 4
  • 19

4 Answers4

0

Checking for legal characters is an ideal use case for regular expressions, see for example https://docs.oracle.com/javase/tutorial/essential/regex/, in particular character classes: https://docs.oracle.com/javase/tutorial/essential/regex/char_classes.html.

Konrad Höffner
  • 11,100
  • 16
  • 60
  • 118
0

Your algorithm always sets the isIllegalUsername flag to false if the current character is not the last character in the legalCharacters collection. This happens because you don't stop the inner loop once you've found a legal character matching the current character, e.g.:

boolean isLegalUsername = false;
...
if (test.equals(legalCharacters.get(z))) {
  isLegalUsername = true;
  break;
}

Apart from that, you can easily skip the second for-loop by using List.contains() and setting the flag according to the result.

if(!legalCharacters.contains(test)) {
  isIllegalUsername = true;
} 

Or, better yet, use regular expression as stated in other answers.

Cecilya
  • 519
  • 1
  • 5
  • 20
0

I suggest to step back here. You are already doing down a dangerous path.

You keep addition one little validation here; and another one there. Your code is already in pretty bad shape. I would suggest a much more OOP solution, something like this.

First, create an interface like:

public interface Validator() {
   public void check(String content) throws ValidiationException;
}

And then you implement each validation in its own class that implements this interface.

Then you do something like:

public class DataChecker {
  private final static List<Validator> validators = Arrays.asList(new LengthValidator, new BlaValidator, new ... );

and actual validation turns into:

try {
  for (Validator v : validators) {       
    v.check(someString);
  }
  // passed all checks
} catch (ValidationException v) {
  // v contains details such as: a nice error message to give to the user!

And for the actual implementation of these little classes; refer to the other answers you got. But as said: you should not only focus on the functionality you need to implement, but on the readability and maintainability of the code you are writing!

GhostCat
  • 137,827
  • 25
  • 176
  • 248
0

Here is a solution that uses the brute force method you are trying:

class Main {
    public static boolean isValid(String value) {
        String legalCharacters = "abcdefghijklmnopqrstuvwxzy0123456789";
        boolean valid = true;
        if (value.length() < 3 || value.length() > 15) {
            valid = false;
        }
        else {
            // For each character in the value
            for (int x = 0; x < value.length() ; x++) {
                boolean found = false;
                // Look for the character in legalCharacters
                for (int z = 0; z < legalCharacters.length(); z++) {
                    char c = value.charAt(x);
                    c = java.lang.Character.toLowerCase(c);
                    // If we found the character
                    if (c == legalCharacters.charAt(z)) {
                        // Remember we found it
                        found = true;
                    }
                }
                // If we did not find it
                if (!found) {
                    // This is an invalid value
                    valid = false;
                    // Break out of the outer for loop
                    break;
                }
            }
        }
        return valid;
    }

    public static void main(String[] args) {
        String username = "aba";
        String password = "aba";
        System.out.format("is %s valid: %b%n", username, isValid(username));
        System.out.format("is %s valid: %b%n", password, isValid(password));
        username = "###";
        password = "@@@";
        System.out.format("is %s valid: %b%n", username, isValid(username));
        System.out.format("is %s valid: %b%n", password, isValid(password));
    }
}

Output

is aba valid: true
is aba valid: true
is ### valid: false
is @@@ valid: false

Try it on repl.it