Dylan Dylan - 1 year ago 55
Java Question

Issue with or operator in Java - ||

I'm writing a password validation program and having some issues with the or statement that checks for special characters. I'm not sure what is wrong with it, the rest of the program seems to function properly, but entering a password that matches the criteria, for example, P@ndas123$%, just results in an infinite loop.

I believe that the problem is related to my big statement for the !pass.contains("") portion. If that is right, could someone help me understand why the statement is incorrect?

import java.util.Scanner;

public class DylanJacksonProg5 {
public static void main(String[] args) {
Scanner user_input = new Scanner(System.in);
boolean valid;
valid = true;

do {

//Password rules declared to user
System.out.println("Password Verifier");
System.out.println("Enter a password that meets the following rules: ");
System.out.println(" Is at least 8 characters long");
System.out.println(" Contains at least 1 lower letter character");
System.out.println(" Contains at least 1 upper letter character");
System.out.println(" Contains at least 1 numeric digit");
System.out.println(" Contains at least 1 special character of the following set: !@#$%^&*");
System.out.println(" Does not contain the word 'and' or the word 'the'");

//Gathering password from user
System.out.println("Enter your password: ");
String pass;
pass = user_input.next();

//Validating password to rules
if (pass.length() < 8) {
int shortlen;
shortlen = 8 - pass.length();
System.out.println("Password is too short. Please add " + shortlen + " characters");
valid = false;

} else
System.out.println("Password length is acceptable.");

//Check for character cases
boolean hasUppercase = !pass.equals(pass.toLowerCase());
boolean hasLowercase = !pass.equals(pass.toUpperCase());

if (!hasUppercase) {
System.out.println("Must have an uppercase Character");
valid = false;

if (!hasLowercase) {
System.out.println("Must have a lowercase Character");
valid = false;

if (hasUppercase) {
System.out.println("Has an upper case character");
if (hasLowercase) {
System.out.println("Has a lowercase Character");

//Check for digit
for (int i = 0; i < pass.length(); i++) {
char x = pass.charAt(i);
if (!Character.isDigit(x)) {
valid = false;


//check for special character
if (!pass.contains("!") || !pass.contains("@") || !pass.contains("#") || !pass.contains("$") ||
!pass.contains("%") || !pass.contains("^") || !pass.contains("&") || !pass.contains("*"))
valid = false;
System.out.println("Password requires a special character from the following: !@#$%^&*");

// check for 'and'
if (pass.contains("and")) {
valid = false;
System.out.println("Cannot contain 'and'");

//check for 'the'
if (pass.contains("the")) {
valid = false;
System.out.println("Cannot contain 'the'");

while (!valid);

Answer Source

I'll tell you one problem straight up front.

Since you set valid to true before the validation loop, and only ever set it to false within the loop, entering an invalid password as your first attempt will result in the loop never exiting.

The setting of valid to true should be the first thing done within the loop.

On top of that, your checks each individually apply to every character in the input string, a situation which is impossible. For example, you check whether the entire string is equal to its lowercased variant, then you check whether it's also equal to its uppercased variant.

That means any string with a letter in it will be deemed invalid since xyZZy cannot be equal to both xyzzy and XYZZY at the same time.

It would be more correct to do something like (pseudo-code, and cut down to a few conditions just to show the method):

    hasLowercase = false
    hasUpperCase = false
    hasNumeric = false
    isLongEnough = false

    get string from user

    if len(string) >= 8:
        isLongEnough = true

    for each character in string:
        if character is lowercase:
            hasLowerCase = true

        if character is uppercase:
            hasUpperCase = true

        if character is numeric:
            hasNumeric = true

    isValid = hasLowerCase && hasUpperCase && hasNumeric && isLongEnough
until isValid

This checks each individual character so that any of them (rather than requiring that all of them) will mark a condition as true.

Recommended from our users: Dynamic Network Monitoring from WhatsUp Gold from IPSwitch. Free Download