28

To read a file in Android from your app's private storage area you use the functionopenFileInput().

My question is, is there a way to check if this file exists before calling this function? The function can throw a FileNotFoundException, but I feel like calling this and then doing something based on a try-catch is a bad practice.

Using File.exist() seems like a strange thing to use also since it would require instantiating a class and I am not sure if just passing the name of the file to it would get it to find the file in the private area of my phone.

Alexander Abakumov
  • 13,617
  • 16
  • 88
  • 129
Pieces
  • 2,256
  • 5
  • 25
  • 39

3 Answers3

58
public boolean fileExists(Context context, String filename) {    
    File file = context.getFileStreamPath(filename);
    if(file == null || !file.exists()) {
        return false;
    }
    return true;
}

EDIT:

Also, here is another way for files in external storage.

String fileUrl = "/appname/data.xml";
String file = android.os.Environment.getExternalStorageDirectory().getPath() + fileUrl;
File f = new File(file);

if(f.exists())
return;
Michael Celey
  • 12,645
  • 6
  • 57
  • 62
coder_For_Life22
  • 26,645
  • 20
  • 86
  • 118
  • 2
    Note: using File.exists() in this way 1) doesn't guard against all exceptions, 2) leaves you open to race conditions, and 3) makes you program slower on average ... unless non-existent files are a *likely* scenario. – Stephen C Sep 19 '12 at 02:35
  • 1
    i prefer the first method, as it keeps the opacity as to where the file actually is – njzk2 Jan 14 '13 at 16:57
32

The function can through a FileNotFoundException, but I feel like calling this and then doing something based on a try catch is bad practice.

I disagree. IMO, it is testing if the file exists before opening that is bad practice. Compare these two versions of the code:

File f = new File("someFile");
InputStream is;

Version #1

if (f.exists()) {
    is = new FileInputStream(f);
    ...
} else {
    System.err.println("Doesn't exist");
}

Version #2

try {
    is = new FileInputStream(f);
    ...
} catch (FileNotFoundException ex) {
    System.err.println("Doesn't exist");
}

There a number of problems with the first version:

  • Version #1 makes an extra system call when you call f.exists(). This makes the first version slower on average, unless there is a high probability that the file won't exist.

  • Version #1 has a race condition. If some external process deletes the file at roughly the same time, you could end up with file.exists() returning true, and then the FileInputStream constructor throwing FileNotFoundException. This is the kind of race condition that can be exploited to break security if the file in question is security critical.

    (Actually, there's a second race condition too. If you call file.exists() while the file is being created, it may return false. In that case you will print the error message, even though new FileInputStream could have succeeded. This race condition is probably harmless.)

A further problem is that the FileInputStream is declared as throwing IOException. Testing to see if the file exists only deals with one of the possible failure modes. Your code is going to have to deal with the other IOExceptions anyway.


@Pieces commented:

Exceptions should be for when something actually goes wrong that you have no control over. In this case, I have complete control over it.

Actually, you don't have complete control over it. Certainly not in the general case. Even in your particular use case, the race condition is still possible in theory.

But the real problem with this line of thinking is that you end up jumping through hoops to avoid exceptions in situations where exceptions / exception handling are the best solution. This makes the code more complicated, less readable, and potentially slower and/or more fragile.

The normal dogma goes like this:

"Exceptions should only be used in exceptional situations".

This is not the same as saying what you said. The word "exceptional" really just means "not normal". This has a much broader meaning than "something actually goes wrong that you have no control over".

I tend to expand on the dogma as follows:

  • Exceptions should not be used for normal flow control.

  • Exceptions should not be used if they are going to prove too expensive on average.

  • Exceptions should be used if the tests you would use to avoid them are not reliable.

  • Exceptions should be used if the tests you would use to avoid them are too expensive on average.

  • Exceptions should be used if they significantly simplify your code (modulo the above). And the criterion for simplicity is whether the code is readable by an average Java programmer.

(Note - "on average" and "too expensive" ... and that "normal" is subjective.)

Now one can argue until the cows come home about how exceptional an event needs to be, but my take is that this is really a matter of balancing the relative simplicity of the approaches (in the context) versus the average performance costs (in the context). Any dogmatic rule that doesn't account for the trade-offs and the context is going to do you harm in some cases.

Stephen C
  • 698,415
  • 94
  • 811
  • 1,216
  • But this is normal flow control. – Nora Söderlund Jul 30 '23 at 15:18
  • 1
    @NoraSöderlund - Read the last paragraph. The cows have come home. – Stephen C Jul 30 '23 at 16:43
  • 1
    @NoraSöderlund Checking if a file exists before doing something with it is not only a race condition, it's fundamentally **wrong**. "Do **X** to see if **Y** will work" is a **broken** way of thinking. **X** is not **Y**, so whether or not **X** succeeds or fails does **not** tell you if **Y** will work or not. So what if the file exists if you don't have permissions to read or write to it - or whatever you want to do. So what if the file *doesn't* exist but you don't have permissions to create it. – Andrew Henle Jul 30 '23 at 18:06
0

This work for me.

try {
  FileInputStream fis = openFileInput(String filename);
  // ... do something
try {
    fis.close();
    } catch (IOException e) {
      e.printStackTrace();
    }
  }
} catch (FileNotFoundException e) { 
  e.printStackTrace();
}

but happens, some times, it returns an Exception the same ... working with adb.

rfellons
  • 656
  • 12
  • 28