Mattias Sjögren said:
Anders,
Checking File.Exists is no excuse for not being prepared to hande a
FileNotFoundException. Just because a file exists at one point doesn't
mean it will still exist when you try to do something with it.
I don't agree, and I know about the atomicity problem (the fact that the
file may disappear between your Exists test and the open call).
My guidelines are the following:
1) You MUST use a try/finally (or better a "using" block) to guarantee that
the file will be closed in a predictable way.
2) In normal circumstances (99.99% of the time), you should NOT introduce
any catch clause around file open logic, you should let exceptions percolate
to a generic catchall that reports and logs all exceptions.
3) There are situations where you should test with Exists before opening and
there are other situations where you should NOT test with Exists.
4) There are some VERY RARE situations where you need both the Exists test
and a try/catch, but normally you don't.
Some words of explanation:
1) I am sure that everybody will agree about this one.
2, 3, and 4). It all boils down to how you draw the line between a "normal"
context of execution and an "abnormal" one.
Here are some assumptions that I consider to be true in a "normal" context
of execution:
a) The vital files that my installer has placed in the Program Files area
have not been deleted by the user. If they have, the user will have to fix
it by restauring the file or reinstalling the application.
b) There is no "sweeper" program that runs in the background and deletes
files in an unpredictable way. So, if my application creates a file and then
tries to open it a few seconds later, the file should still be there. If
there is such a sweeper program then the user should identify the culprit
(virus?) and put his system back into a stable mode.
c) The application may use some optional files. In this case, the
application will have to test if the file exists or not before opening it,
and the application will have a fallback strategy (like using some default
values instead of values read from the file).
d) There may be special situations where the application will use files to
exchange data with another application. If the synchronization protocol
between the two apps is weak, the application must be ready to handle the
case where the file gets deleted by the other app in unpredictable ways (for
example between a call to Exists and a call to open).
So, here are the coding patterns that I recommend in some common scenarios:
When the application needs to open a "vital" file created by the installer,
or a temp file that another method has created just before (cases a and b),
the correct pattern is:
using (Stream stream = File.Open(filename, ...)) {
DoSomething(stream); }
When the application need to work with an "optional" file (case c), the
correct pattern is:
if (File.Exists(filename))
using (Stream stream = File.Open(finename, ...)) {
DoSomething(stream); }
else
UseDefaultValues();
In the very special situation described in d) above (and I would not
recommend using this kind of interprocess communication but sometimes this
is the only way to deal with legacy apps), The correct pattern is:
using (Stream stream = MyFileHelper.TryOpen(filename, ...))
{
if (stream != null)
DoSomething(stream);
}
where MyFileHelper.TryOpen is coded as:
Stream TryOpen(filename, ...)
{
if (!File.Exists(string filename))
return null;
try {
return File.Open(filename, ...);
}
catch (FileNotFoundException ex)
{
return null;
}
}
With these guidelines, you get simple, understandable application code
(instead of a big mess of try/catch), and you know exactly the assumptions
that the developer makes about the files (is it a file that should always
exist? and if it does not, this is just as abnormal as if the disk was bad,
or is it an optional file? or is it a file that may disappear in
unpredictable ways?).
In short, the pattern that you will be using depends on the assumptions that
you make about your operating environment. In many cases, the simplest
pattern (just opening the file without any Exists test nor any catch clause)
is the one to use.
Bruno.