3

This is a question about exception handling and prevention.

public static string PathCombineNoEx(string path1, string path2)
{
   if (path1 == null || path2 == null /*Either validate here*/)
   {
      return null;
   }

   try
   {
      return System.IO.Path.Combine(path1, path2);
   }
   catch (ArgumentException /*or catch here*/)
   {
      return null;
   }
}

Since exceptions are an enormous hit on performance we should try to minimize the chance for exceptions to be thrown. In the following example I've eliminated the chance that Path.Combine could throw an ArgumentnullException. This was very easy to do and does almost not affect performance in any way. However, Path.Combine also throws an ArgumentException if one of the two parameter strings contains any invalid character provided by GetInvalidPathChars.

  1. Now, would you recommend to catch this as I did or would you really check for invalid chars before calling the Path.Combine?
  2. What about a general recommendation that can be applied to most situations.
  3. Maybe there is a Microsoft article about that?

Path.Combine documentation:
https://msdn.microsoft.com/de-de/library/fyy7a5kt(v=vs.110).aspx

The .NET Reference Source:
http://referencesource.microsoft.com/#mscorlib/system/io/path.cs,2d7263f86a526264

Microsft performance tip (see chapter Throw fewer exceptions):
https://msdn.microsoft.com/en-us/library/ms973839.aspx

Noel Widmer
  • 4,444
  • 9
  • 45
  • 69

3 Answers3

3
  1. Catching exceptions is slow since exception throwing does stack trace.
  2. Catching exceptions is less readable; it's a kind of notorious goto: if something has happened then goto catch.

That's why I vote for validation:

   if (path1 == null) 
     return null;
   if (path2 == null) 
     return null; 

   //TODO: put other validations here, e.g. Path.GetInvalidFileNameChars()  

   return System.IO.Path.Combine(path1, path2);

And catch exceptions for exceptional cases only:

   try {
     // I can't validate this, since just after I've finished it and ready to read 
     // someone can
     //   - delete/rename the file
     //   - change permissions
     //   - lock file (e.g. start writing to it) 
     String data = File.ReadAllText(@"C:\MyData.txt"); 
     ...
   }
   catch (IOException e) {
     ... 
   }
Dmitry Bychenko
  • 180,369
  • 20
  • 160
  • 215
  • @Noël Widmer: Yes I didn't *catch* `ArgumentException` of `Path.Combine`: instead, I suggesting validation: check for `Path.GetInvalidFileNameChars()`, `Path.GetInvalidPathChars()` and other if required – Dmitry Bychenko Apr 29 '16 at 07:30
  • I know how to do it. Just wanted to know if you'd still validate it. thanks :) – Noel Widmer Apr 29 '16 at 07:56
2

Exceptions, as the term says, are meant to handle unexpected situations. I vote to handle foreseeable cases in code beforehand.

Thorsten Dittmar
  • 55,956
  • 8
  • 91
  • 139
0

Exceptions can hit performance.

If it's an API,

public static string PathCombineNoEx(string path1, string path2)
 {
  if (String.IsNullOrWhiteSpace(path1))
  {
    throw new ArgumentnullException(path1);
  }

 //Same goes for Path2

 return System.IO.Path.Combine(path1, path2);
}

Otherwise, Dmitry's answer will do.

Helpful SO posts:

Business Objects, Validation And Exceptions

Why are Exceptions said to be so bad for Input Validation?

Community
  • 1
  • 1
mlg
  • 1,162
  • 1
  • 14
  • 32