2
public Path createPath(String name){
    return Files.createFile( Paths.get( name ) );
}

In createPath(String name), the java.nio.file.Files throws a java.io.IOException. This is however to be used by a larger library. And I mean huge. If I just add the exception to the method signature, that would mean that the larger library would also need to add the exception to every method using it. Wrapping it in a try catch would supposedly save the day, but that would make the code damn ugly. (in my eyes):

public Path createPath(String name){
    Path file = null;
    try {
        file = Files.createFile( Paths.get( name ) );
    } catch ( IOException e ) {
        e.printStackTrace();
    }
    return file;
}

What would be the best approach for this?

  • I'm new here. I've just started as an associate software engineer :D Maybe this question isn't so good, please feel free to downvote. But I would greatly appreciate it if I were redirected to sources where I can get answers. Thanks! – Rigo Sarmiento Aug 1 '18 at 14:13
  • Note that you hiding the possible failure of producing a File by returning either a File or null instead. That's terrible: every caller now has to be aware that the Object they were expecting from the library might unexpectedly be null instead, which requires a totally different reaction. Essentially, you're forcing all callers to program exception handling by hand, rather than using the more efficient built-in mechanism. – Kilian Foth Aug 1 '18 at 14:20
  • I know the method is terrible but that wasn't the purpose of this question, this was but a sample method so I can easily state my problem. Yes, I've mentioned the consequence of adding the exception to the method signature. So i'm asking for the best approach. The try/catch? – Rigo Sarmiento Aug 1 '18 at 14:27
  • 2
    Ah, the joys of checked exceptions in Java. – Robert Harvey Aug 1 '18 at 15:19
3

It looks like you have two options in mind:

  1. createPath swallows the exception and the application crashes somewhere on NullPointerException.
    • Actually, the correct way to do it is to throw a RuntimeException or an Error or a subclass of one of them.
  2. Add throws IOException to the entire callchain up to main and let the application crash with IOException.

But the actual intention behind checked exceptions (or exceptions in general) is different:

  1. Add throws IOException to the callchain up to the caller that knows what to do in case a file can not be created, and that caller will catch the IOException and handle it.

Consider the following example:

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Scanner;

public class App {
    private Scanner reader;

    public App() {
        reader = new Scanner(System.in);
    }

    public String getPathFromUser() {
        System.err.print("Enter a path: ");
        return reader.nextLine();
    }

    public Path createPath(String name) throws IOException {
        return Files.createFile(Paths.get(name));
    }

    public Path promptForPath() {
        while (true) {
            String path = getPathFromUser();
            try {
                return createPath(path);
            } catch (IOException e) {
                System.err.printf("Unable to create %s (%s)\n", path, e);
                continue;
            }
        }
    }

    public static void main(String[] args) {
        App app = new App();
        Path path = app.promptForPath();
        // do stuff with path
    }
}

Here main calls promptForPath which in turn calls createPath.

  • promptForPath knows how to handle that IOException (it asks the user for a different path), so it doesn't need to be marked with throw IOException.
  • createPath doesn't know how to handle it, so it must be throw IOException.
  • main doesn't know how to handle it either - but it doesn't have to because promptForPath already handled it.

Is it better to pollute the methods in the call stack with throws clauses or to uglify createPath with try...catch?

These things are only ugly and cumbersome when put in the wrong place. When put in the right place, rather than being ugly pollution they can actually fit in as part of the logic.

The try...catch in the createPath in the question is an ugly code-smell - it is clear it is there to hide a problem. The try...catch in my promptForPath, on the other hand, is not ugly because it is handling a problem.

Similarly, throws IOException in the top level methods is ugly - why would your main be throwing an IOException? But in the lower methods that get the path it makes sense ("this method deals with paths and may encounter a non-existing path") so they are not ugly - they are helpful parts of the signatures.

Another thing to consider: if you bubble the exceptions all the way up, you are just replacing one exception with another. Not sure if it's worth the work. But if you are only bubbling it up to the point where you can handle it, the quality of your application is improved - instead of crashing, for example, you are notifying your user that the path they entered is invalid and prompt them to enter a new one. This is a preferable behavior from the user's end, and makes the refactor worthwhile.

  • You haven't addressed the OP's primary concern, which is that handling a checked exception higher up in the call stack means that every method in the call stack must now have a throws clause. – Robert Harvey Aug 1 '18 at 15:24
  • @RobertHarvey but I did - I wrote that not every method in the call stack must have that throws clause, only the ones up to the point where the exception can be handled. – Idan Arye Aug 1 '18 at 16:05
  • 1
    Well, yes, that's what I meant. The op needs to know whether or not he should take that cost, rather than some alternative like re-throwing with an unchecked exception. Nobody argues that you don't need event handling to prevent the application from crashing. – Robert Harvey Aug 1 '18 at 16:12
  • Oh, I get it. I'll edit my answer. – Idan Arye Aug 1 '18 at 16:36
4

First and foremost, the correct answer has to address this one simple question:

What guarantees does this method have to make?

In short, what are the requirements? If you are requesting the method to create a file, and it does not create that file, then it is not satisfying its purpose. The reason it can't create that file can be any number of reasons ranging from permissions, to the path you are supposed to create the file in does not exist--or even cannot exist.

At this point, you have 3 options:

  • Ignore the problem--sometimes it's not a problem, but don't just ignore something and hope it goes away. It won't.
  • Return a status indicating success or failure, potentially what kind of failure.
  • Throw an exception that the parent library has to catch. Of course the next question is whether to declare a checked exception or use a runtime exception.

In this case, if you can't create the file, the owning library may be able to recover from that condition and attempt to create a different file. Or it might not be recoverable, and it needs to do some cleanup before it informs the end code using that larger library that something is indeed very wrong.

By silently ignoring the problem, you are making the library assume the operation completed successfully when it did not. Invalid assumptions are the cause of any number of subtle and not so subtle errors. You want to clearly communicate the success or failure of the operation so that the code that uses that function can operate on known state.

The question of whether the exception is checked or not really is one of what kind of pain you want to endure:

  • Checked exception is an exception that must be handled or explicitly passed along. This has the immediate pain of deciding what to do with the exception in your code now.
  • Runtime exception is an exception that doesn't need to be declared, but you can catch it if you need to. (In languages other than Java, this is usually the only kind of exception supported). The pain you endure for this is when the exception happens and it is unhandled, it crashes the application using your larger library.

You'll have to make your decisions based on how likely you think the exception will be thrown and how much of an impact to the system it will be if the exception is not handled directly.

  • The fourth option is to use checked exceptions the way they were meant to be used and put a throws clause on every method in the call stack. – Robert Harvey Aug 1 '18 at 15:26
  • @RobertHarvey, yeah, that's technically included in throw an exception even though I didn't specifically call it out. Almost forgot about Java's painful peculiarity on exception handling. – Berin Loritsch Aug 1 '18 at 15:27
-1

Clearly the 'best' approach is the throw the exception and alter the library accordingly.

What are the alternatives? :

  • catch and throw an already declared Exception
  • catch and throw an unchecked Exception
  • catch and return null

All have their own problems and are 'less good' than the full refactor

  • You haven't really provided any rationale here. All you say in your answer is what you think the best approach is, but you don't explain why. – Robert Harvey Aug 1 '18 at 15:21
  • because theres no arguement about what's 'best' the only question is do you have time to do the best solution. – Ewan Aug 1 '18 at 15:26
  • Then your answer needs to explain why it's still better to take the time. – Robert Harvey Aug 1 '18 at 15:27
  • i dont think so. everyone knows java forces your to declare exceptions and why – Ewan Aug 1 '18 at 15:31
  • "Everyone knows" is not a satisfactory answer on any SE site. – Robert Harvey Aug 1 '18 at 15:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.