Whats wrong with this method?

Question

Here's the method:

public static String CPUcolor () 
{ 
    System.out.println ("What color am I?") ; 
    String s = getIns() ; 
    System.out.println ("are you sure I'm "+s+"? (Y/N)") ; 
    String a = getIns() ; 
    while (!((a.equals ("y")) || (a.equals ("Y")) || (a.equals ("n")) || (a.equals ("N")))) 
        {
            System.out.println ("try again") ; 
            a = getIns () ; 
        } 
    if (a.equals ("n") || a.equals("N"))
        {CPUcolor() ;} 
    System.out.println ("I am "+s) ;
    return s ; 
}

here is a possible output of this method (y's and n's are user inputs):

What color am I?
red
are you sure I'm red? (Y/N)
N
What color am I?
blue
are you sure I'm blue? (Y/N)
N
What color am I?
Yellow
are you sure I'm Yellow? (Y/N)
y
I am Yellow
I am blue
I am red

Why is it that the line's "I am blue" and "I am red" printed? Why are they printed in reverse order with red, the first entered, printed last?

Answer 1

That's simple recursion. You are calling CPUcolor() within your CPUcolor() again. When the call returns, the remaining commands of each original method will be executed.
To fix it you have to add a return:

if (a.equals ("n") || a.equals("N"))
{
  return CPUcolor();
}

Answer 2

Note that

    if (a.equals ("n") || a.equals("N"))
        {CPUcolor() ;} 
    System.out.println ("I am "+s) ;

should be:

    if (a.equals ("n") || a.equals("N"))
        {CPUcolor() ;} 
    else
        {System.out.println ("I am "+s) ;}

This way you only print the color in the single instance when the user actually answered Yes (you do not want to print the color for those instances when the user answered No, instances which you revisit in reverse order as you unwind your recursion -- the reason for the reverse order in which the other answers were printed.)

Also note that you do not need (nor want) recursion in this particular example: once you add the else your method becomes tail-recursive, and you can achieve the same effect iteratively. By eliminating recursion you also eliminate a vulnerability problem, that is, the possibility of a malicious user entering No indefinitely until your program eventually crashes with a StackOverflowException:

public static String CPUcolor () 
{ 
  while (true) {
    System.out.println ("What color am I?") ; 
    String s = getIns() ; 
    System.out.println ("are you sure I'm "+s+"? (Y/N)") ; 
    String a = getIns() ; 
    while (!((a.equals ("y")) || (a.equals ("Y")) || (a.equals ("n")) || (a.equals ("N")))) 
        {
            System.out.println ("try again") ; 
            a = getIns () ; 
        } 
    if (a.equals ("y") || a.equals("Y")) {
      System.out.println ("I am "+s) ;
      return s ; 
    }
  }
}

Answer 3

Because you're calling the new CPUColor() before printing out the results of this one.

Answer 4

I have indented the output, to help make it a little bit clearer, what's happening:

What color am I?
red
are you sure I'm red? (Y/N)
N
    What color am I?
    blue
    are you sure I'm blue? (Y/N)
    N
        What color am I?
        Yellow
        are you sure I'm Yellow? (Y/N)
        y
        I am Yellow
    I am blue
I am red

Every level of indentation is one level deeper in the call hierarchy: one more call to CPUColor(). After a call to CPUColor() returns, the rest that follows still has to be done.

I like to view it similarly to the folders in a file directory tree: Just imagine folding and expanding the lower levels of directories!