CODEWITHSUNDEEP
javaexception
chill appreciator
chill appreciator

Reputation: 432

What is the proper way to return a value from try-catch block?

An example that does not work due to the lack of a return value:

public Path writeToFile() {
    try {
        Path tempFilePath = Files.createTempFile(Paths.get(""), "sorting_test_", ".txt");
        BufferedWriter bw = new BufferedWriter(new FileWriter(tempFilePath.toFile()));

        for (List<Integer> arr : arrays) {
            // Convert array ints to strings, join it to single string and write
            bw.write(arr.stream()
                    .map(String::valueOf)
                    .collect(Collectors.joining(" ")));
            bw.newLine();
        }
        bw.close();

        return tempFilePath;
    } catch (IOException e) {
        e.printStackTrace();
    }
}

I know that I can do like this:

public Path writeToFile() {
    Path tempFilePath = null;
    //try-catch{...}
    return tempFilePath;
}

But it looks ugly. Is there a more natural way to solve this task?

Upvotes: 0

Views: 488

Answers (5)

Alexander Pogrebnyak
Alexander Pogrebnyak

Reputation: 45606

Another solution, instead of eating an IOException (antipattern), convert it to an appropriate subclass of RuntimeException and throw from the catch block.

Also, in your example, you are leaking file handler, by not closing FileWriter on exception.

public Path writeToFile() {

    final Path tempFilePath;
    try {
        tempFilePath = Files.createTempFile(Paths.get(""), "sorting_test_", ".txt");
    } catch (IOException e ) {
        throw new MyRuntimeException(
                "Cannot create sorting_test temp file",
                e
            );
    }

    try (final FileWriter fw = new FileWriter(tempFilePath.toFile())) {
        try(final BufferedWriter bw = new BufferedWriter(fw)) {
            for (List<Integer> arr : arrays) {
                // Convert array ints to strings, join it to single string and write
                bw.write(arr.stream()
                    .map(String::valueOf)
                    .collect(Collectors.joining(" ")));
                bw.newLine();
            }
        }

        return tempFilePath;
    } catch (IOException e) {
        throw new MyRuntimeException(
                "Cannot write to " + tempFilePath,
                e
            );
    }
}

Upvotes: 1

user9827968
user9827968

Reputation:

The most appropriate way is to keep the return statement in try block.

If we keep the return statement in finally or after catch we might be swallowing the exception.

This is an old link that seems to be related. See if this helps.

Upvotes: 0

Stephen C
Stephen C

Reputation: 719576

Here are some possible solutions:

  • Change the method signature to public void writeToFile(). Don't return the Path. (But this probably won't work for you: you probably need the Path.)

  • Add return null; at the end of the method. This has the disadvantage that the caller needs to deal with the case where null is returned ... or else it will get NPEs when they attempt to use the non-existent Path.

    This is equivalent to your "ugly" solution. It is debatable which is better from a stylistic perspective. (A dogmatic "structured programming" person would say your way is better!)

  • Change the signature to return as Optional<Path>. This is a better alternative than returning an explicit null. If you implement it correctly, the caller is effectively forced to deal with the "absent" case.

  • Remove the try catch and change the signature of the method to public Path writeToFile() throws IOException. The caller has to deal with the checked exception, but that may be a good thing!

I should point out that your code is not handling the resources properly. You should be using try with resources to ensure that the stream created by FileWriter is always closed. Otherwise there is a risk of leaking file descriptors that could ultimately result in unexpected I/O errors.

Upvotes: 2

shah06
shah06

Reputation: 11

I don't know why you're looking for a more "natural" solution, but you could just return null in your catch block.

Upvotes: 1

Ryuzaki L
Ryuzaki L

Reputation: 40078

If you don't want to return null i will prefer using Optional from java 8

public Optional<Path> writeToFile() {
    try {
        Path tempFilePath = Files.createTempFile(Paths.get(""), "sorting_test_", ".txt");
        BufferedWriter bw = new BufferedWriter(new FileWriter(tempFilePath.toFile()));

        for (List<Integer> arr : arrays) {
            // Convert array ints to strings, join it to single string and write
            bw.write(arr.stream()
                    .map(String::valueOf)
                    .collect(Collectors.joining(" ")));
            bw.newLine();
        }
        bw.close();

        return Optional.of(tempFilePath);
    } catch (IOException e) {
        e.printStackTrace();
    }
   return Optional.empty()
}

So in the caller method you can use

public void ifPresent(Consumer consumer)

or

public boolean isPresent()

Upvotes: 1

Related Questions