6
\$\begingroup\$

I've seen a similar question to this but mine's a little more specific. I have the below

File f = new File("./data.txt");
if(f.isFile())
{
    try
    {
        FileChannel fc = new RandomAccessFile(f,"r").getChannel();
        //Get information and return it
    }
    catch(FileNotFoundException e)
    {
        System.out.println("FileNoFoundException: "+e.getMessage());
    }
}
else
{
    //If the file wasn't found, return default values
}

I was wondering, should I just put the default returning of the information in the catch block?
I assumed that'd be a no because you're not really supposed to use it to control flow. But having the both the if and the try catch just looks too redundant to me.

\$\endgroup\$
  • 3
    \$\begingroup\$ Exceptions control flow whether you like it or not. You shouldn't throw Exceptions to control normal program flow, that doesn't mean that you should avoid controlling the program flow when you catch the exception. \$\endgroup\$ – Simon Forsberg Jan 22 '14 at 18:07
  • \$\begingroup\$ The if(f.isFile()) statement looks redundant to the FileNotFoundException. Also, what if f is null? \$\endgroup\$ – bobobobo Jan 23 '14 at 2:39
  • \$\begingroup\$ Yeah, the redundancy was what I was unsure of. Whether I should do things based on if an exception is thrown or if I should take steps using the if statement (perhaps proactive is the right word) to check the file because having both looks weird. I did not consider f being null. What might go wrong to cause that? Out of memory or something similar? \$\endgroup\$ – Coburn Jan 23 '14 at 14:21
4
\$\begingroup\$

In cases like this, I like taking a default-unless-better approach... Consider this re-working of your code to do the same thing in a different order.

The basic premise is:

  1. set up a default value (or null if it is a complicated thing to do).
  2. do the hard work which may be missing dependencies or may fail
  3. if the hard work completes successfully, return that result.

The reason this works well is because you can exit from the hard work at any point, and have the default standing by to continue.....

... also, as an exercise, use the try-with-resources and new NIO2 features in Java7

// set up the default value...
SomeValue result = null;
Path path = Paths.get("./data.txt");
if (Files.isRegularFile(path)) {
    try (SeekableByteChannel channel =
            Files.newByteChannel(path, StandardOpenOptions.READ)) {

        // do the work required for your file....
        ...

        // after everything is successful... set the result:
        result = new SomeValue(....real arguments ....);
    } catch (IOException ioe) {
        System.out.println("FileNoFoundException: " + e.getMessage());
    }
}
result == null ? new SomeValue(defaults...) : result;

Additionally, there is no reason, if you are in a method that builds these things, that you can't return immediately with the right answer.

Often if you are doing things like the above, it indicates that what you are doing should be in a sub-method, with an early-return when you have a successful setup, and a default return value when things fail.

\$\endgroup\$
  • \$\begingroup\$ It actually is in a submethod and after reworking it a bit I found that this was the best option. Thanks! Very similar to the answer below but I understood this one more. \$\endgroup\$ – Coburn Jan 23 '14 at 14:24
4
\$\begingroup\$

My usual pattern here is to set the default values first, then replace the defaults with the expected values in the happy path.

int resultA = DEFAULT_FOR_A;
String resultB = DEFAULT_FOR_B;

 File f = new File("./data.txt");
 if(f.isFile())
 {
    try
    {
        FileChannel fc = new RandomAccessFile(f,"r").getChannel();
        resultA = computeA(fc);
        resultB = readB(fc);
    }
    catch(FileNotFoundException e)
    {
        // TODO: replace with a logging framework
        System.out.println("FileNoFoundException: "+e.getMessage());
    }
}

// use resultA and resultB as required.

If computeA() and readB() can also throw, then you need to think about whether the results are coupled or not. If they are coupled, then I would advise storing the results of the functions in variables scoped to the try block, and only overwriting the results after you are certain that no further exceptions will be thrown.

\$\endgroup\$

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.