Handling Expected Exceptions

Question

I read several articles regarding good practices in exception handling. Most of it tackled unexpected exceptions yet expected by the author. I just want to clarify and eliminate possible bad practices that I could be doing. Since I already expect these problems to happen already, I assume throwing an exception is a bit redundant.

Let's say I have this code :

string fileName = Path.Combine(Application.StartupPath, "sometextfile.txt");

// There's a possibility that the file doesn't exist <<<<<<<<<<<<<<<<<<<<<
if (!File.Exists(fileName))
{
   // Do something here
   return;
}

// Therefore, this will return an exception
using (StreamReader file =
     new StreamReader(fileName))
{
     // Some code here
}

Naturally, what I would do is to inform the user with a MessageBox saying "File not found". Is there an efficient or a better way of doing this?

Another idea that I have is to create an enum which contains expected error codes then create a method which will call a MessageBox showing the error message for that specific situation :

enum ErrorCodes {null, zero, ...}
public void showError(ErrorCodes error)
{
    string message;
    switch (error)
        {
        case ErrorCode.null:
        {
             message = "value cannot be null";
             break;
        }
        case ErrorCode.zero:
        {
             message = "cannot divide by zero";
             break;
        }
}

MessageBox.Show(message, Application.ProductName, MessageBoxButtons.OK, MessageBoxIcon.Error);
}

Answer 1

If the code that is trying to access the file is on the front end, for example an event handler for an on-click, then it's OK to check for the error situation, display a message and return.

If I understand your question correctly you want to know whether you should do this:

public void button_Click() {
    if(!File.Exists(textBox.Text)) {
        MessageBox.Show("Could not find the file");
        return;
    }

    ProcessFile(textBox.Text); // would have thrown an exception if the file didn't exist
}

That would be fine, except if ProcessFile throws any other kind of Exception it won't be handled.

You could do this:

public void button_Click() {
    try {
        ProcessFile(textBox.Text); // throwns an exception if the file didn't exist
    } catch(Exception ex) {
        MessageBox.Show(GetUserMessage(ex));
        return;
    }
}

In my opinion it's better to do both:

public void button_Click() {
    try {
        if(!File.Exists(textBox.Text)) {
            MessageBox.Show("Could not find the file");
            return;
        }

        ProcessFile(textBox.Text); // throwns an exception if the file didn't exist
    } catch(Exception ex) {
        MessageBox.Show(GetUserMessage(ex));
        return;
    }
}

This way you can provide the most specific message to the user relevant to what he was doing at this point. For example if he was trying to open an Excel file you could say "Could not find the Excel file you wanted to import".

This also works in case the file was deleted or renamed between the point you checked and the point you tried to process the file.

Alternatively you could accomplish something similar with this:

public void button_Click() {
    try {
        if(!File.Exists(textBox.Text)) {
            throw new UserException("Could not find the file");
        }

        ProcessFile(textBox.Text); // throwns an exception if the file didn't exist
    } catch(Exception ex) {
        MessageBox.Show(GetUserMessage(ex));
        return;
    }
}

In this case you would create your own Exception class UserException and just pass that message along without translating it. This would allow you to reuse the same code you use to display a message.

Exceptions in Classes

If the error occurs in some class library then you should throw an exception. The purpose of an exception is that an error can't go unnoticed.

For example you shouldn't want this:

class MyFileHandler {
    public void OpenFile(string fileName) {
        if(!File.Exists(fileName)) return;
        // do stuff
    }

    public void DoStuff() {
        // do stuff
    }
}

Now if a developer called myFileHandlerInstance.OpenFile("note.txt") he would assumed it worked. You could return a boolean, like so:

class MyFileHandler {
    public bool OpenFile(string fileName) {
        if(!File.Exists(fileName)) return false;
        // do stuff
        return true;
    }

    public void DoStuff() {
        // do stuff
    }
}

But now you are relying on the developer checking that value, this used to be a common method but errors got ignored and overlooked which is why Exceptions became better practice.

As far as what to display to the user, you really shouldn't display the Exception message directly, those messages are intended for developers not users. I suggest a method that takes an exception object and returns the best message, like so:

public string GetUserErrorMessage(Exception ex) {
    if(ex is FileLoadException) {
        var fileLoadException = (FileLoadException)ex;
        return "Sorry but we failed to load the file: " + fileLoadException.FileName;
    }
}

You can inspect the Exception properties for details including error codes if you like. Also I suggest capturing the actual exception details somewhere for your own debugging purposes, somewhere where it's not visible to the user.

Answer 2

These things are never easy. By definition an exception is (like the Spanish Inquisition) never expected. In your first example the file might exist when you call File.Exist, but then not exist when you try to open it (file deleted, network failed, thumb drive pulled, etc.). As far as a best practice: protect yourself where you can, catch only the exceptions you know how to handle and hope for the best. Programs will fail and the very best thing you can do is make sure that unhandled failures are communicated as clearly as possible to the user of the code (the heart of your second example).

Communicating a failure depends greatly on the what the code does and who is using it. MessageBoxes are often used but generally not 'useful', because if the cause of the error isn't clear to the user about the only thing they can do is send you a jpg image (that's a best case :-D). Providing a way to log problems and/or a custom dialog that allows users to cut/paste information (like a stack trace) is much more useful. Think about the case where a user selects a file and there is a network hiccup. They tell you they got this error about the file not being found, but when you or they look it's there because the network is working correctly by that time. A simple "file not found" message does give you enough information and so you have to tell the user "I don't know, glad it works now" ... this does not inspire confidence. Best practice here is to leave yourself enough 'breadcrumbs' to at least have a rough idea about what has happened. Even something like having a full file path can give you the clues you need to figure out what went wrong.