I am trying to implement retry-mechanism using a recursive method. Although the method already returned the list, the consecutive lines of code are executed as well.

I am sure I am missing something very obvious which a trained eye can spot in a second. How can I rework the recursion, so I get rid of the last return null statement. It's not needed at all.

This is the entire code:

import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.*;

public class Retryer {

    private static final int RETRY_COUNT = 3;
    private static final long RETRY_DELAY = 2; // in seconds

    private static int tryCounter = 0;

    public Retryer() {}

    public void execute() {
        List<String> downloadedFiles;
        try {
            Callable<List<String>> c = new AsyncFileDownloader();
            downloadedFiles = c.call();
        } catch (Exception e) {
            System.out.println("Ops! Starting retry");
            ScheduledExecutorService scheduledExecutorService = Executors.newSingleThreadScheduledExecutor();
            downloadedFiles = retry(scheduledExecutorService, 0);
            System.out.println("Retry ended");
            if (downloadedFiles == null || downloadedFiles.isEmpty()) {
                System.out.println("Sorry. Will exit now");
                System.exit(0);
            }
        }

        System.out.println("Finished successfully with "   downloadedFiles.size()   " in list.");
    }

    private List<String> retry (ScheduledExecutorService ses, int retryCounter) {
        if (retryCounter == RETRY_COUNT) {
            System.out.println("Retry limit reached. Exiting");
            ses.shutdown();
            return null;
        }
        try {
            System.out.println("Retry attempt # "   (retryCounter 1)   " will start in "   RETRY_DELAY   " seconds ...");
            ScheduledFuture sf = ses.schedule(new AsyncFileDownloader(), RETRY_DELAY, TimeUnit.SECONDS);
            List<String> downloadedFiles = (List<String>) sf.get();
            ses.shutdown();
            System.out.println("In retry. success "   downloadedFiles );
            return downloadedFiles;
        } catch (Exception e) {
            System.out.println("--recursive calling");
            retry(ses,   retryCounter);
        }
        System.out.println("--returning null!!! This shouldn't happen");
        return null;
    }

    class AsyncFileDownloader implements Callable<List<String>> {
        @Override
        public List<String> call() throws Exception {
            synchronized (this) {
                tryCounter  ;
                System.out.println("--in pull");
                if (tryCounter < 3) throw new IOException("Some bad thing happened");
                List<String> retList = new ArrayList<>();
                retList.add("file1");
                retList.add("file1");
                return retList;
            }
        }
    }
}

And here the output:

--in pull
Ops! Starting retry
Retry attempt # 1 will start in 2 seconds ...
--in pull
--recursive calling
Retry attempt # 2 will start in 2 seconds ...
--in pull
In retry. success [file1, file1]
--returning null!!! This shouldn't happen
Retry ended
Sorry. Will exit now

CodePudding user response：

If the first execution goes into the catch block, you recursively call retry. As soon as this second call returns, you continue executing the code after the catch block in the first call, leading to you hitting the print "returning null".

What you actually want is probably to return the value of the recursive call. Remember that return returns a value to the function that called another function, not necessarily the first function in a chain of calls.