Data validation in Java

Question

Well let's just say that this used to work fine then I started it up again today and now it doesn't....

pin is 1234 and no matter what I do it says it's not valid...

and yes I know that it doesn't check the third time. I have to fix that too:

    import java.util.Scanner;

    import java.util.Scanner;


  public class ATM
  {

      public ATM()

      {
          Scanner console = new Scanner(System.in);

          final String pin = "1234";
          String userPin = "";
          int pinCount = 1;
          boolean error = false;

          do{
              System.out.print("Enter PIN: ");
              userPin = console.nextLine();

              if (pinCount == 3) {
                  System.out.println("Bank account is blocked");
                  break;
              }
              else if (userPin.length() < 4 || userPin.length() > 4) {
                  error = true; 
                  pinCount += 1;
              }
              else if (isNumeric(userPin) && userPin == pin && pinCount < 3) {
                  System.out.println("Your PIN is correct");
                  break;
              }
              else{
                  System.out.println("Your PIN is incorrect");
                  error = true;
                  pinCount += 1;
              }
          }while(error);          
      }

      public static boolean isNumeric(String str)
        { 
          try  
          {  
            double d = Double.parseDouble(str);  
          }  
          catch(NumberFormatException nfe)  
          {  
            return false;  
          }  
          return true;  
        }
  }

Don't ask me why I have it split:

public class ATMtest 
{
    public static void main(String[] args) 
    {
        ATM atm = new ATM();
    }

}

Any help would be greatly appreciated.

`if (userPin.length() < 4 || userPin.length() > 4)` should be simplified to `if (userPin.length() != 4`. Also, the `isNumeric` check is superfluous. — Robert, Nov 16 '13 at 18:34

score 4 · Accepted Answer · answered Nov 16 '13 at 18:31

4

You don't compare String objects with ==. You should use the equals() method. I know it does not make sense if you are new in java but == means "are they the same reference?" and equals() means "are they equal?".

answered Nov 16 '13 at 18:31

Adam Arold

29,285
22
112
207

@user2999980 This is why your code sometimes works and sometimes doesn't. String literals MAY refer to the same memory location, but don't necessarily. See the javadoc on String.intern() if you're curious. – Robert Nov 16 '13 at 18:58

score 2 · Answer 2 · answered Nov 16 '13 at 18:31

2

userPin == pin should be userPin.equals(pin)

answered Nov 16 '13 at 18:31

Robert

139
4

Masudul · Answer 3 · 2013-11-16T18:41:26.047

For digit match with exact 4 length use \d{4} regex and remove all boilerplate code on if else statement for validation.

public static boolean isNumeric(String str) {
    Pattern p = Pattern.compile("\\d{4}");
    Matcher matcher = p.matcher(str);
    return matcher.matches();

}

For String comparison use equals method inted of == . Change it from

else if (isNumeric(userPin) && userPin == pin && pinCount < 3) {

To

else if (isNumeric(userPin) && userPin.equals(pin) && pinCount < 3) {

score 1 · Answer 4 · answered Nov 16 '13 at 18:35

You declare two String variables in your code:

  final String pin = "1234";
  String userPin = "";

and you validate them whether they are equal to or not.

  else if (isNumeric(userPin) && userPin == pin && pinCount < 3) {
          System.out.println("Your PIN is correct");
          break;
      }

String comparison should be used equals method rather than ==

Change

userPin == pin

To

 userPin.equals(pin)

score 0 · Answer 5 · answered Nov 16 '13 at 18:37

Change this as

  else if (isNumeric(userPin) && userPin == pin && pinCount < 3) {
                      System.out.println("Your PIN is correct");
                      break;
                  }

this

else if (isNumeric(userPin) && userPin.equals(pin) && pinCount < 3) {
                      System.out.println("Your PIN is correct");
                      break;
                  }

score 0 · Answer 6 · answered Nov 16 '13 at 18:38

0

Would a switch not be better here instead of all those if conditions?

answered Nov 16 '13 at 18:38

Peter_James

647
4
14

You have switch on very specific conditions. So you could do a switch on userPin.length or even on the value of userPin, but that would only work if you're looking for very specific values. else...if is the correct structure. – Robert Nov 16 '13 at 18:57

Data validation in Java

6 Answers6