I made a switch case statement menu with one of the options being System.exit(0);. This is all surrounded by a try, finally that calls the method all of this is in. Would you guys not recommend this style of loop or am I all good?

public void Run() {
    Scanner myObj = new Scanner(System.in);
    int menuInput;
    
    try {
    System.out.println(" 1) call something\n"
         "2) quit");
    
    menuInput = myObj.nextInt();
    myObj.nextLine();
    
    switch(menuInput) {
        case 1:
            something();
            break;
        case 2:
            System.exit(0);
            break;
    }
    
    }catch (Exeption e ){
        System.out.println("Something went wrong.");
    
    }finally{
        Run();
    }
}

CodePudding user response：

No.

What you have here is an infinite recursion. Eventually you'd overflow the stack.

Use an actual loop instead:

while (true) {
  try {
    // ...
  } catch (Exception e) {
    // ...
  }
}

And you almost never want to call System.exit. Just break the loop instead.

CodePudding user response：

Is this legal code? Yes.

Is what you have there recommended? No.

If the method throws an exception it's likely recalling it will throw again. See the quote below.

Calling it again in a tight loop without attempting remedy, at least waiting a recovery and counting failures (3 strikes out?) will just end up in a tight loop of failure and stack overflow here.

So:

Can you identify errors that retrying may work and only retry on those?

You should almost certainly include some kind of 'back-off' wait before retry.

Always (always!) include a maximum retry number after which you accept failure.

In my experience the only kind of failure that may work on retry is 'service unavailable' meaning an intermittent outage.

It may not be relevant, but things like (say) invalid credentials aren't going to fix themselves and ideally you don't resubmit those. That's particularly because you end up locking the account and being in an even worse state and possibly causing issues for others using the valid credential...

The other scenario is (say) file not found and you're using the non-existence of a file as a way of polling for something.

That's a poor design pattern and is a misuse of exception handling.

You should strongly prefer to use some kind for existence check in those cases and not let routine activity get confused with exception handling of issues.

Also if you do retry log each attempt (it may be useful later to see whether things are running smoothly or getting delayed in retry scenarios even if the go through eventually). But always differentiate a 'Warning' when retrying and 'Error' when 'throwing in the towel' and failing.

public static class Runner {
    
    private static int MAX_RETRIES=3;
    private static int BACK_OFF_MILLIS=30000;
    
    public void Run() throws Exception,InterruptedException {
        final int TRIES=3;//In reality may be configured.
        int trycount=1;
        for(;;){
            try{
                tryRun();
                return;
            }catch(Exception e){
                String message=e.getMessage();
                if(trycount>=MAX_RETRIES){
                    System.out.println("*FAILED*: " e.getMessage());
                    throw e;
                }
                boolean retriable=true;
                //Any tests for non-retriable exceptions here...
                if(!retriable){
                    System.out.println("*FAILED*: non-retriable exception - " e.getMessage());
                    throw e;
                }
                  trycount;   
                System.out.println("Warning: " e.getMessage() " retrying "  trycount " of " TRIES);
                try {
                    Thread.sleep(trycount*BACK_OFF_MILLIS);//Some kind of back-off...
                }catch(InterruptedException ie){
                    System.out.println("*FAILED*: Interrupted. Aborting.");
                    throw ie;
                }
                continue;
            }
        }
    }
    
    public void tryRun() throws Exception{
        //Real workload goes here!
    }
}

NB: The back-off strategy here is very simplistic. When it comes to outages then it's usually recommended to implement a random element and an increasing back-off like 1 minute, 10 minutes, 25 minutes. But that's a topic in itself.

I'm not sure who really said but this popular quote seems relevant.

The definition of insanity is doing the same thing over and over again and expecting different results