How to put this duplicated code into a method using parameters?-CodePudding

I am fairly new to Java programming and have been prompted to do some refactoring where the logic for this code is the same and just needs to be put into a method that can be called and have the services passed as parameters. The functionality is basically to shut the threads down we have two thread pools here and just need to know how I could pass these services as parameters they are apart of an interface as well. Here is the code that I am trying to refactor into a method that can be called.

if (scheduledExecutorService1 != null) {
            scheduledExecutorService1.shutdown(); // Disable new tasks from being submitted
            try {
                // Wait a while for existing tasks to terminate
                if (!scheduledExecutorService1.awaitTermination(60, TimeUnit.SECONDS)) {
                    scheduledExecutorService1.shutdownNow(); // Cancel currently executing tasks
                    // Wait a while for tasks to respond to being cancelled
                    if (!scheduledExecutorService1.awaitTermination(60, TimeUnit.SECONDS))
                        System.err.println("Pool did not terminate");
                }
            } catch (InterruptedException ie) {
                // (Re-)Cancel if current thread also interrupted
                scheduledExecutorService1.shutdownNow();
                // Preserve interrupt status
                Thread.currentThread().interrupt();
            }
        }
        if (scheduledExecutorService2 != null) {
            scheduledExecutorService2.shutdown(); // Disable new tasks from being submitted
            try {
                // Wait a while for existing tasks to terminate
                if (!scheduledExecutorService2.awaitTermination(60, TimeUnit.SECONDS)) {
                    scheduledExecutorService2.shutdownNow(); // Cancel currently executing tasks
                    // Wait a while for tasks to respond to being cancelled
                    if (!scheduledExecutorService2.awaitTermination(60, TimeUnit.SECONDS))
                        System.err.println("Pool did not terminate");
                }
            } catch (InterruptedException ie) {
                // (Re-)Cancel if current thread also interrupted
                scheduledExecutorService2.shutdownNow();
                // Preserve interrupt status
                Thread.currentThread().interrupt();
            }
        }

CodePudding user response：

Simply put the code into a method:

private void xx(type_of_scheduledExecutorService scheduledExecutorService ) {
    if (scheduledExecutorService != null) {
        scheduledExecutorService.shutdown(); // Disable new tasks from being submitted
        try {
            // Wait a while for existing tasks to terminate
            if (!scheduledExecutorService.awaitTermination(60, TimeUnit.SECONDS)) {
                scheduledExecutorService.shutdownNow(); // Cancel currently executing tasks
                // Wait a while for tasks to respond to being cancelled
                if (!scheduledExecutorService.awaitTermination(60, TimeUnit.SECONDS))
                    System.err.println("Pool did not terminate");
            }
        } catch (InterruptedException ie) {
            // (Re-)Cancel if current thread also interrupted
            scheduledExecutorService.shutdownNow();
            // Preserve interrupt status
            Thread.currentThread().interrupt();
        }
    }
}

and call it twice:

xx(scheduledExecutorService1);
xx(scheduledExecutorService2);

CodePudding user response：

With any refactoring, it becomes a question of "what do you have" and "what do you need". We can see the if statements are essentially duplicated, so let's literally pull that out and work with it (noticing the removal of variable1 and variable2, we're being generic!):

public void ourMethod() {
    if (scheduledExecutorService != null) {
        scheduledExecutorService.shutdown(); // Disable new tasks from being submitted
        try {
            // Wait a while for existing tasks to terminate
            if (!scheduledExecutorService.awaitTermination(60, TimeUnit.SECONDS)) {
                scheduledExecutorService.shutdownNow(); // Cancel currently executing tasks
                // Wait a while for tasks to respond to being cancelled
                if (!scheduledExecutorService.awaitTermination(60, TimeUnit.SECONDS))
                    System.err.println("Pool did not terminate");
            }
        } catch (InterruptedException ie) {
            // (Re-)Cancel if current thread also interrupted
            scheduledExecutorService.shutdownNow();
            // Preserve interrupt status
            Thread.currentThread().interrupt();
        }
    }
}

Now, you'll notice plenty of errors in the above code doing it literally like that. However, these errors answer the immediate question of "what do you need": a modern IDE will highlight those missing variables! So we extract that information out into parameters for our new method, giving it an appropriate name:

public void handleShutdown(ScheduledExecutorService service) {
    if (service != null) {
        service.shutdown(); // Disable new tasks from being submitted
        try {
            // Wait a while for existing tasks to terminate
            if (!service.awaitTermination(60, TimeUnit.SECONDS)) {
                service.shutdownNow(); // Cancel currently executing tasks
                // Wait a while for tasks to respond to being cancelled
                if (!service.awaitTermination(60, TimeUnit.SECONDS))
                    System.err.println("Pool did not terminate");
            }
        } catch (InterruptedException ie) {
            // (Re-)Cancel if current thread also interrupted
            service.shutdownNow();
            // Preserve interrupt status
            Thread.currentThread().interrupt();
        }
    }
}

Now it compiles, and we don't seem to need any return value. So the call is now simply:

handleShutdown(scheduledExecutorService1);
handleShutdown(scheduledExecutorService2);

If your executors were held in a List or some other data structure, you could forgo the "index-naming" of the variables, and simply:

list.forEach(MyClass::handleShutdown);