Dzejms
Dzejms

Reputation: 3258

Should a File Delete Operation Throw an Exception if the File Doesn't Exist

The answer may be context dependent or this may not be the right place, so let me know if https://softwareengineering.stackexchange.com/ is a more suitable site.

We're re-working all of the file I/O in our app to be programmed to an interface instead of the existing tightly coupled System.IO implementation. The current code has lots of File.Exists() checks before calling File.Delete(). We have the opportunity to put the File.Exists() check in the implementation that deals with the System.IO and remove them from the consuming code, so something like

private IFileSystem fileSystem;

void methodName()
{
    string filePath = "c:\\file.txt";
    if (fileSystem.Exists(filePath)
        fileSystem.Delete(filePath);
}

could simply become:

private IFileSystem fileSystem;

void methodName()
{
    string filePath = "c:\\file.txt";
    fileSystem.Delete(filePath);
}

The question is, is there any reason that fileSystem.Delete() should throw an exception if the file doesn't exist? We can't think of a good reason for our domain, but it seems strange to not throw an exception when we're so used to the framework API behaving a certain way. We aren't a super experienced bunch with different languages and platforms, so we don't much to compare this to.

It would be nice, from the code consumer's perspective, to just delete the file and not have to check every time. That seems like a win. But are there any known dangers with this line of thinking?

EDIT*

I should add that the whole point of the operation is to allow us to talk to Azure BLOB storage, where the APIs are different. So staying close to .NET conventions in System.IO isn't essential.

Upvotes: 2

Views: 662

Answers (3)

Hans Passant
Hans Passant

Reputation: 942448

Plenty of reasons for File.Delete() to throw, particularly when you try to delete a file in the C:\ directory. But no, not because it doesn't exist.

Pretty important to understand why it works this way. From the way your snippet looks, you are making a huge mistake by relying on File.Exists() too much. Which is a very weak guarantee on a multi-tasking operating system. All that you know is that a file did/didn't exist at the moment in time that Exists() ran. It says absolutely nothing about the file state after that. Another process may well delete or create the file a microsecond later. By not needing the check, you can safely delete the file without that race condition crashing your program.

Note that File.Exists() is evil in many other cases as well. Like only opening a file after checking that it exists. That just can't work reliably for the same reason, another process may of course delete the file right between the check and the attempt to open it. The only way to be sure is by opening it without checking. If it wasn't there then the exception tells you about it.

If this sounds a lot like a thread race problem then you're right, that's exactly what it is. But usually without a way to use anything like a lock statement to ensure that another process doesn't screw you up. It is possible, you could use a named Mutex to arbitrate access to file. The typical problem is that you'll have to deal with a process that doesn't know beans about it.

Upvotes: 3

paparazzo
paparazzo

Reputation: 45106

Why would you swallow potentially critical information?
What if the delete was throwing an UnauthorizedAccessException because the program was being run under different credentials?
What if the file had a lock that was preventing the delete?
Don't assume bad unexpected stuff is not going to happen.

Upvotes: 0

usr
usr

Reputation: 171246

The caller might want to know whether a file existed or not. Return this information in a status enum or a bool. Exceptions are for unexpected errors or even usage errors. This is a totally expected case.

I think the .NET file system APIs get this wrong a lot. They do not pass on all the information that Win32 makes available. Often, you cannot even find out what error occurred without parsing the exception text.

Upvotes: 0

Related Questions