user3363742
user3363742

Reputation: 1

Java - Boolean method always returning as false

 public static boolean isValidNumber(String a1) 
  {

    String x = ("0123456789");
    boolean valid = false; 

    for (int i = 0; i < 4; i++) {
    char c = a1.charAt(i);

      for (int j = 0; j < 10; j++) {
          if ( c == x.charAt(j)) {
            valid = true;
          }
          else {
            valid = false;
          }
        }
      }

      return valid;
  }

The above method checks to see whether an input of a four character string is composed of the characters 0123456789. However, regardless of what the input is, the method always returns as false.

If I were to change the valid value in the else statement to true, the method would always return as true.

What is the error that I have made in this method?

Upvotes: 0

Views: 2399

Answers (5)

John3136 is quite correct, but I would like to propose even better solution to your whole task:

final static String x = "0123456789";
public static boolean isValidNumber(String a1) {
    for (int i = 0; i < a1.length(); ++i) {
        if (x.indexOf(a1.charAt(i)) == -1) return false;
    }
    return true;
}

In short: the above code "looks up" every character in your parameter string a1 in the string composed of digits. If it can find it, continues. If it can't, it means a1 consist not only digits and returns false. If it passes through all a1 characters then it returns true :)

And as asked and described in the comments - handling of duplicate characters in argument string:

final static String x = "0123456789";
public static boolean isValidNumber(String a1) {
    for (int i = 0; i < a1.length(); ++i) {
        final char currentChar = a1.charAt(i);
        if (x.indexOf(currentChar) == -1 || a1.indexOf(currentChar, i+1) != -1)
            return false;
    }
    return true;
}

The function call a1.indexOf(currentChar, i+1) essentially checks if there is any duplicate character in the rest of the string (from position i+1 and farther). Which means if it will be able to find duplicate char, the method return false :) Hope this helps, here is more info on String.indexOf(int, int) function if you want:

http://docs.oracle.com/javase/7/docs/api/java/lang/String.html#indexOf(int, int)

Upvotes: 1

Sanjeev
Sanjeev

Reputation: 9946

You can use this one liner function to check for validity of a String as Number using Regular Expression

 public static boolean isValidNumber(String a1) 
 {
     return a1.matches("[\\d]+");
 }

Hope this helps.

Upvotes: 0

Karthik Kalyanasundaram
Karthik Kalyanasundaram

Reputation: 1525

I would rewrite your function as given below,

String x = ("0123456789");
boolean valid = false; 

for (int i = 0; i < 4; i++) {
   char c = a1.charAt(i);

   boolean isCharOK = false;
   for (int j = 0; j < 10; j++) {
      if ( c == x.charAt(j)) {
         isCharOK = true;
         break;
      }
   }

   if (!isCharOK) {
      valid = false;
      break;
   }
}

return valid;

Upvotes: 1

stinepike
stinepike

Reputation: 54672

a String.equals method will check these two strings in a single statement if you are permitted to use that.

 public static boolean isValidNumber(String a1) 
  {

    String x = ("0123456789");
    return x.equals(a1);
  }

Upvotes: 1

John3136
John3136

Reputation: 29266

As soon as you find a non matching character, break the loop otherwise the next matching character will set valid to true.

e.g. "123a456" is considered valid.

for (int j = 0; j < 10; j++) {
    if ( c == x.charAt(j)) {
        valid = true;
    }
    else {
        valid = false;
        break;
    }
}

If for some reason you don't want to break the loop, you could keep an "invalid counter" and make sure that is 0 at the end.

Of course for what you are doing here, Integer.parseInt() might be your best bet ;-)

Upvotes: 2

Related Questions