-2

Normally I do my code along the lines of this for validation:

public static void Menu()
{
    Scanner keyboard = new Scanner(System.in);
    if (!keyboard.hasNextInt())
    {
        System.out.println("Incorrect input, try again");
        Menu();
    }  
    else
    {
        // switch statement etc
    }
}

I am just wondering is this bad practice? And if so why and what are better ways of doing it besides using recursion. I have used recursion for getting the power of numbers and some other things so I understand the idea of it.

xlecoustillier
  • 16,183
  • 14
  • 60
  • 85
Jim
  • 13
  • 1

5 Answers5

9

This is recursion, and this a bad practice in this case, as you'll end with a stack overflow exception if you input incorrect data too many times. Try a while loop instead:

  Scanner scanner = new Scanner(System.in);

  int choice = 0;

  while (scanner.hasNext()) {
     if(scanner.hasNextInt()) {
         choice = scanner.nextInt();
         break;
     }

     System.out.println("Incorrect input, try again");
     scanner.next();
  }

  scanner.close();

  // switch statement etc
  switch(choice) {
      //...
  }

This will create only one Scanner instance, and will keep running smoothly until some valid value gets entered.

Keep recursion for cases when these conditions are met:

  • if it increases code readability/maintainability
  • if there's no risk of stack overflow (you know that the recursive code won't execute more than a limited number of times)

Performance is also usually better with loops, but if you use recursion, you should most of the time meet the stack overflow long before you notice a significant performance loss.

xlecoustillier
  • 16,183
  • 14
  • 60
  • 85
  • 1
    But would this not keep outputting "Incorrect input, try again" over and over until a int is typed? Does it say it once and then wait for input? When I tried this and typed a letter it kept repeating the incorrect error thousands of times. – Jim Sep 06 '13 at 12:13
  • 1
    Can't find the article, but I also read the JIT compiler cannot optimize recursion (except for tail recursion) so it's best to avoid it if possible unless you take the JIT into consideration. – Jared Beekman Sep 06 '13 at 12:44
  • @JaredBeekman If you can find the reference, it would be particularly useful here, because in the OP's code, the recursive call to `Menu()` _is_ in tail position. If the compiler _can_ optimize that, then this is essentially a loop, and the extra Scanner allocation could be eliminated by making the Scanner an argument to the function. – Joshua Taylor Sep 06 '13 at 14:21
  • After digging for about an hour I can't find the original article I read, but I did find a bunch of articles/posts stating that (this may have changed since some of them are 'old') even tail call recursion may not be optimized due to the JVM requiring full access to the call stack. This [post](http://stackoverflow.com/questions/72209/recursion-or-iteration#72522) has a good discussion and references. It would seem that Java 8 may be way different in the way it handles recursion though with the addition of Lambda expressions. – Jared Beekman Sep 06 '13 at 18:56
  • That said the OP should be able to send a scanner, instantiated prior to the function call, in as an argument and just use the fact that it's a reference to prevent multiple instantiation's and then just remember to close it afterwards (or depending on the version of Java use the try-with-resources scheme). – Jared Beekman Sep 06 '13 at 18:59
1

In this case, I would say Yes.

  1. You declare a new Object in this method. Think of Memory!
  2. In this case a while-loop would do a better job

edit:// to slow

Danny.
  • 363
  • 1
  • 4
  • 23
0

This type of recursion is bad when the number of recursive calls are not fixed. You will probably get OutOfMemoryError exception after some wrong attempts. So better find an alternative or if you have to use the recursion like this, put a counter which allows you certain number of times attempts.

Vimal Bera
  • 10,346
  • 4
  • 25
  • 47
0

The main point about using recursive algorithms isn't so much a mthod calling itself but more like a method returning some result to itself for further processing: there is a call-stack that is build up but at some depth (hopefully) the recusion terminates and results are passed back up the stack to generate some final result.

Your example is just building up a calling stack by repeatedly calling Menu() from itself but there is no passing back of results, so in my opinion it's not proper recursion at all but it will only fill up your stack with useless clutter.

piet.t
  • 11,718
  • 21
  • 43
  • 52
-2

Loops are always better than recursion. In recursion the internal stack is built up. Also recursion requires a method sub routine jump which is slower than loops

user2670866
  • 88
  • 2
  • 14