For loop not executing

[Sohail Hasware]

I am very new in Java and I am attempting some exercises that I have created for myself. The console program I have gives the user a maximum of three attempts to login. The input works properly once, for both a passed or failed login and printing out a appropriate message. In both cases though I get an error after the first attempt. The user is set to "raja" and password to "k9"
Here is my code:

import java.util.Scanner;

public class login {
	
	public static void Scan()
	{
		   String user;
	       String password;
	       for (int i=1; i<3; i++)
			{
	       Scanner in = new Scanner(System.in);
	       

	       System.out.print("Enter your user name: ");
	       user = in.nextLine();
	       
	       
	      System.out.print("Enter your password: ");


	       password=in.nextLine();
	       in.close();            

	       Cred(user,password);
	       
	       if(i==3)
	       {
	    	   System.out.println("You had three login attempts");
	       }
			}

	    }
	

	public static void Cred(String n, String p)
	{
		
		if (n.equals("raja") && p.equals("k9"))
		{
			
			System.out.println("Welcome " +n);
		}
		else
		{
			System.out.println("You dont have valid credentials");
			Scan();
		}
		}
		
	

	public static void main(String[] args)

            {
                Scan();
            }
}

Thanks
-Sohail

[fred rosenberger]

What would that error message be? Those things actually contain a lot of good information on what the problem is.

General debug advice would be to use a ton of System.out.println() statements all over the place to see what exactly your code is doing.

[Sohail Hasware]

For correct credentials I get the following error:

Enter your user name: raja
Enter your password: k9
Welcome rajaException in thread "main"
Enter your user name: java.util.NoSuchElementException: No line found
at java.util.Scanner.nextLine(Scanner.java:1533)
at login.Scan(login.java:15)
at login.main(login.java:78)

And for incorrect credentials I get the following error:

Enter your user name: yhtt
Enter your password: tyuu
You dont have valid credentialsException in thread "main"
Enter your user name: java.util.NoSuchElementException: No line found
at java.util.Scanner.nextLine(Scanner.java:1533)
at login.Scan(login.java:15)
at login.Cred(login.java:46)
at login.Scan(login.java:24)
at login.main(login.java:78)

[Jeff Verdegan]

Create your Scanner once, outside the loop, and keep it open until you're completely done reading for the whole app.

[Sohail Hasware]

Here is the rewritten code:

public static void Scan()
	{
		   String user;
	       String password;
	       Scanner in = new Scanner(System.in);
	       for (int i=1; i<3; i++)
			{
	       System.out.print("Enter your user name: ");
	       user = in.nextLine();
	       
	       System.out.print("Enter your password: ");


	       password=in.nextLine();
	                   
	       Cred(user,password);
	       
	       if(i==3)
	       {
	    	   System.out.println("You had three login attempts");
	       }
			}
	       in.close();
	    }

For correct credentials the reply is:

Enter your user name: raja
Enter your password: k9
Welcome raja
Enter your user name:

For improper credentials the reply is:

Enter your user name: rr
Enter your password: rr
You dont have valid credentials
Enter your user name: tt
Enter your password: tt
You dont have valid credentials
Enter your user name: yy
Enter your password: yy
You dont have valid credentials
Enter your user name:

In both cases the first attempt goes through this time with out errors but halts. The system outputs the proper response in both cases but then prompts for the first scan "Enter your user name: " The if statement for a improper login is never executed after three login attempts.

[fred rosenberger]

follow the logic of your code...

you start in main(). That calls Scan(). That calls Cred(). Cred calls Scan (line 46 of your original post).

That does not jump you back to your ORIGINAL scan method, but calls a NEW version..so you now have a second version of Scan() running. That creates a new Scanner, a new i (which gets initialized to 1) and starts the process over.

basically, you have a recursive loop...Scan calls Cred which calls Scan which calls Cred which calls Scan...until you hit the correct sequence to start exiting back up to the first one. I'm guessing you get tired before then.

So the question is...Why do you have Cred() calling Scan()?

and note: Class names should be capitalized, so this should be "public class Login". Methods should start with a lowercase, so these should be scan() and cred().

[Sohail Hasware]

My shaky logic behind that is that if the credentials a wrong this code

System.out.println("You dont have valid credentials");
			Scan();

will call the Scan() method so the user can attempt a second or third time and final time. Sorry, I was totally unaware of that, the way I have written my code, a new version of the Scan() method gets created and therefore a new Scanner Class.
How should I proceed with my logic that the user gets three attempts, for each failed attempt the user is sent back to the original Scan() method and prints "You dont have valid credentials" and after three failed attempts gets the last print "You had three login attempts" If the login is correct the is greeted by "Welcome Raja"

I also appreciate the naming advice you suggested.

Thanks

[fred rosenberger]

It seems to me that you haven't thought through this before you started coding. I may be wrong, but I would guess you didn't write out anything longhand first, in English, describing what you want your code to do.

Look at your Scan() method. What does it do? In VERY high level terms, it loops two times (yes, only twice), and asks the user to input some data, and then calls Cred(). Note that the way it is written, 'i' will NEVER equal three inside the loop, so your check on line 18 (in your most recent post) will always be false.

I would suggest this. Start over. keep this code around for reference, but start over.

Think about what Scan should do...it should loop (up to) three times, asking the user for input each time. It should (perhaps) call Cred() to see if the input name/pw are valid (hint: Cred() should probably return something to say if they were). The loop should probably bail out if they were valid, or we have tried three times.

What I would do is write that code in VERY small chunks. When I get to the point where I nee to call Cred(), I would at first write Cred() to be a dummy method - the guts would be empty. all it would do is return a 'success' EVERY time - it should be one line long inside the body. I would then test the heck out of the Scan() method. Eventually, you would change your Cred() method to always return a 'failure' EVERY time, and again test the heck out of Scan().

Only when I am SURE Scan() works how I want would I start thinking about implementing the body of the Cred() method. I wouldn't have Cred() print anything. Its job is ONLY to see if the name/pw are valid, and return a value to the calling method. The rational is that some day, you may want to check the credentials of something else, and it may not make sense to print out "you don't have valid credentials" there. A method should have a specific task and should do that, do it very well, and do nothing else.

[Sohail Hasware]

I rewrote the code. This time the loop is executing three times as it should for three invalid login attempts or displays a welcome message for a successful login. I changed the validation method to a Boolean return value, and cleared all print statements from it.
My problems is reading the value of the i counter variable in the for loop. I want to print the login attemp nr after each failed login. After two failed login attempts I want to also warn the user that this will be the final login attempt. Atfter the third failed attempt I want to print to read the i value and show that there were three failed attempts.

Here is the rewritten code:

import java.util.Scanner;

public class Logon {
	
	public static void scan()
	{
		   String user;
	       String password;
	       
	       Scanner in = new Scanner(System.in);
	       for (int i=0; i<3; i++)
			{
	    	   if(i!=0)
	    	   {
	    	   System.out.println("Invalid user: That was attempt nr " +i);
	    	   }
	    	   else
	    		   
	    	   {  
	    		   if(i==1)
	    		   System.out.println(+i+ "nd and final attempt");
	    	   }
	    	  
	       System.out.print("Enter your user name: ");
	       user = in.nextLine();
	       
	       System.out.print("Enter your password: ");

	       password=in.nextLine();
	                   
	       isValidCred(user,password);
	       
	       if(isValidCred(user,password))
	       {
	    	   System.out.println("Welcome "+user);
	    	   break;
	       }
	       else
	       {
	    	   if(i==2)
	    	   System.out.println("You attempted to access the system " +i+ " times");
	       }
	       
	       }
	       in.close();
	    }
	
	public static boolean isValidCred(String n, String p)
	{
		
		if (n.equals("raja") && p.equals("k9"))
		{
			return true;
		}
		else
		{
			return false;
		}
		}
			
	public static void main(String[] args) 
	{
	    System.out.println("You have 3 attempts to logon");  
		scan();
	}
}

And the result of a successful login attempt:

You have 3 attempts to logon
Enter your user name: raja
Enter your password: k9
Welcome raja

After three unsuccessful attempts:

You have 3 attempts to logon
Enter your user name: ee
Enter your password: ee
Invalid user: That was attempt nr 1
Enter your user name: ee
Enter your password: ee
Invalid user: That was attempt nr 2
Enter your user name: ee
Enter your password: ee
You attempted to access the system 2 times

As your see this is never executed

{  
	    		   if(i==1)
	    		   System.out.println(+i+ "nd and final attempt");
	    	   }

and this code

if(i==2)
	    	   System.out.println("You attempted to access the system " +i+ " times");

results in

You attempted to access the system 2 times

when the desired result should be "3 times"

Thanks
-Sohail

[fred rosenberger]

Look at this code VERY carefully:

  if(i!=0)  
  {  
     System.out.println("Invalid user: That was attempt nr " +i);  
  }  
  else  
 {    
    if(i==1)  
      System.out.println(+i+ "nd and final attempt");  
 }  
  //more code
}

First, you should have brackets around the body of the "if(i==1)". It is a good practice to ALWAYS use brackets, even if they aren't required.

Tell me under what conditions could line 8 run? If i is not 0, the block on lines 2-4 runs, and the else block from lines 6-9 is skipped.

if i IS 0, we run the code from lines 6-9. That code then says "if i is 1" (which we know it can't be), run line 8...so line 8 will indeed never run.

You could figure that out yourself it you took my previous advice. When the code doesn't work how you think it should, put System.out.println() statements in there. I would have done something like this:

System.out.println("before if, i is " + i)
  if(i!=0)  
  {  
     System.out.println("i is not 0");
     System.out.println("Invalid user: That was attempt nr " +i);  
  }  
  else  
 {  
    System.out.println("i is zero")  ;
    if(i==1)  
      {
      System.out.println(+i+ "nd and final attempt");  
      }
 }  
  //more code
}

As soon as I got that far, I'd see what the problem was...i wouldn't even have to run it.

[fred rosenberger]

if(i==2)  
{
  System.out.println("You attempted to access the system " +i+ " times");  
}

again, this should have curly braces.

So you explicitly test to see if i is 2, and if so, you expect that when you print i you get a 3???

You have two options. I will NEVER be 3 inside your loop. So, you can either do this:

System.out.println("You attempted to access the system " + (++i) + " times");

Which is ugly, or you can declare i outside your loop, and move where you do that printing

int i;
for (i=0; i<3; i++)
{
  //blah blah blah
}
System.out.println("You attempted to access the system " +i+ " times");

another note...you call "isValidCred()", but don't do anything with the result. you then call it again inside your if statement. Call it once, save the result, and use that.

[Sohail Hasware]

I am slowly getting there and actually learning a lot from your advice :P First I have changed the following portion of the code so that it now accommodates

Invalid user: That was attempt nr 2
2nd and final attempt

Changed code:

for (int i=0; i<3; i++)
			{
	    	   if(i!=0)
	    	   {
	    	   System.out.println("Invalid user: That was attempt nr " +i);
	    	   }
	    	   
	    		   if(i==2)
	    		   {
	    		   System.out.println(+i+ "nd and final attempt");
	    		   }
	    	   

I have used the more ugly suggestion of your code for the final print statement

System.out.println("You attempted to access the system " + (++i) + " times");

The reason being that if I declare outside the loop I get the error "Duplicate local variable i" where the for statement is written? Actually previously I was thinking of setting another variable to be equal of i and reading that variable instead when reading in the final nr of attempts. But I couldn't figure what the proper syntax would have been.

Your final suggestion of setting a local variable in my scan() method to the result of isValidCred, use that local variable to call and store the result and then just use that value it is true to break the loop and welcome the validated user is something I am will utilize soon as I understand the proper syntax... but it will be soon :P

[fred rosenberger]

Nobody can be sure what the problem is without seeing your actual code...but my guess is you're doing this:

int i = 0;
for (int i = 0;...blah blah blah)

The problem is that second 'int' inside the 'for'. When you say 'int <whatever>', you are telling the compiler "i want to create a new variable of type 'int' named 'i'". so if you do that outside the for loop, then again inside the for loop, the compiler says "nope...you can't have two variables with the same name".

If you declare it outside the loop, you just use it in the loop....

int i = 0; //you don't really even need to initialize it here...you could just do "int i;"
for (i = 0;....)

[Sohail Hasware]

I think I have finally nailed my code, this is how it looks like now:

import java.util.Scanner;

public class Logon {
	
	public static void scan()
	{
		   String user = null;
		   String password=null;
		   boolean isValid;
		   int i=0;
		   
		   Scanner in = new Scanner(System.in);
		
		   for (i=0; i<3; i++)
		   {
		       if(i!=0)
			   {
			   System.out.println("Invalid user: That was attempt nr " +i);
			   }
			   
				   if(i==2)
				   {
				   System.out.println("This " +(++i) + "rd login will be your final attempt");
				   }
			   //System.out.println("Counter i is: " +i);
		    		   
				   System.out.print("Enter your user name: ");
			       user = in.nextLine();
			       
			       System.out.print("Enter your password: ");
			       password=in.nextLine();
			       
			       isValid = isValidCred(user,password);         
			       if(isValid)
			    	   
			       {
			    	   System.out.println("Welcome "+user);
			    	   break;
			       }
		   
		       
		       if(i==3)
		       {
		       	   System.out.println("You had " + i + " failed attempts");  
		       }
		          
		   	       }//for loop ends here
	      
	       in.close();
	    }    

	
	public static boolean isValidCred(String n, String p)
	{
		
		if (n.equals("raja") && p.equals("k9"))
		{
			return true;
		}
		else
		{
			return false;
		}
		}
			
	public static void main(String[] args) 
	{
	    System.out.println("You have 3 attempts to logon");  
		scan();
	}
}

			       

This is the ouput for a successful login:

You have 3 attempts to logon
Enter your user name: raja
Enter your password: k9
Welcome raja

and after three failed logins:

You have 3 attempts to logon
Enter your user name: ee
Enter your password: ee
Invalid user: That was attempt nr 1
Enter your user name: ee
Enter your password: ee
Invalid user: That was attempt nr 2
This 3rd login will be your final attempt
Enter your user name: ee
Enter your password: ee
You had 3 failed attempts