Guard Clause and Exception handling for the same condition

Question

I ran across the following snippet of code. Names have been changed to protect the innocent:

    public void RunProgram()
 {
  System.IO.FileInfo fInfo = new System.IO.FileInfo(Application.StartupPath + "Program.exe");

  if (!fInfo.Exists)
  {
   System.Windows.Forms.MessageBox.Show("Program could not be found, please verify your installation.\n\nDetails:\n" + fInfo.FullName);
            return;
  }

  try
  {
   System.Diagnostics.Process process = new System.Diagnostics.Process();
   System.Diagnostics.ProcessStartInfo pStart = new System.Diagnostics.ProcessStartInfo(); 
   pStart.FileName = fInfo.FullName;
   pStart.UseShellExecute = true;
   process.StartInfo = pStart;
   process.Start();
  }
  catch
  {
   System.Windows.Forms.MessageBox.Show(string.Format("An error occurred trying to run the program:{0}", fInfo.FullName));
  }
 }

I know there are a few things wrong here:

• Exception types aren't being handled individually
• Error message isn't informative enough

Rest assured I'll be addressing those as well but my main question is about the check for the file's existence just before the try/catch block. This strikes me as a bit redundant.

The point of exception handling is to catch unexpected conditions. I fully expect the file to be there so removing the existence check and just letting the exception handling catch it if it isn't seams a reasonable solution to me.

What do you think?

Answer 1

The file not being there is something that we can reasonably anticipate - and since we are at the UI level (I assume, since MessageBox), it makes reasonable sense to sanity check the request, and just tell the user directly and cancel.

If we were deep in the bowels of the code (a few levels removed from the UI), then an exception would be the right thing - but I'd still probably check for file existance first and give a sensible error. Of course, any file existance check is immediately a thread race condition ;-p

Answer 2

I vote to remove the .Exists() check.

As for handling the exception types, my normal mode is to start with a generic exception handler to catch everything, but also make sure those exceptions are logged. Then break that apart to handle different exception based on the logs.

Answer 3

It is redundant and may give a false sense of security. Please note that you could still get a FileNotFound exception due to concurrency.

Also see: Is there a case where parameter validation may be considered redundant?

Answer 4

I always try to avoid exceptions. This is generally because I tend to run visual studio with break on exceptions enabled, so try to avoid exceptions as much as possible.

I've also spent a significant amount of time working on embedded systems where throwing an exception was expensive. This may not be the case for C#.