Handling exceptions in a property's set/get

Question

In the example code below I am taking a list of strings and concatenating them to a delimited string. The problem occurs when I use the setter with an empty list. The ToString method throws an ArgumentOutOfRangeException since the second parameter is a -1.

How should conditions (exceptions thrown in getters/setters) like this be handled?

I don't like the fact that the setter throws an exception since the caller doesn't know about the internals of the class and therefor should not have to handle (or even know how) the exception. Catching all exceptions in the getter/setter and quietly handling them also sounds like a bad idea since the caller will not know the getter/setter failed.

//I realize that this isn't the best code but I wanted to produce an example
//to demonstrate my question.
private string theStringVariable;
const string DELIMITER = ",";

public IList<string> StringList
{
   set
   {
      StringBuilder stringBuilder = new StringBuilder();
      foreach(string entry in value)
      {
         stringBuilder.Append(entry);
         stringBuilder.Append(DELIMITER);
      }
      theStringVariable = stringBuilder.ToString(0, stringBuilder.Length - 1);
   }
}

Answer 1

You should check for the potential, common error conditions, and throw your own exception (prior to the StringBuilder errors) right up front, with a meaningful error message.

In your case, you'll most likely want to use some form of ArgumentException if the setter is called with an Empty string list. The key here is that your exception can say "the argument contained an empty collection" instead of "index out of bounds", which is going to make the caller understand, immediately, why they have a "real" problem in their call.

---

On a side note: In cases like the code you posted - I'd also consider making this a method instead of a property. You're doing quite a bit of "work" in this property setter, which is going to be somewhat unexpected. By making this a method, you'll be giving the user a clue that there's a bit of "processing" occurring within this property...

Answer 2

I don't think your issues deal with best practices.

The best practice you need to watch in your case is the line that says

 stringBuilder.ToString(0, stringBuilder.Length - 1);

You are causing the exception to be thrown by not checking the string length. If your length is 0, just return empty string.

If we are talking in generalities, if you have the ability to code around common issues, empty sets, badly formatted data, then you should do your best to shield the user from unnecessary errors.

However* sometimes it is better to abruptly/loudly fail then it is to silently fail.

Answer 3

First of all, if you can spot someone that might cause an exception --- fix it so the exception doesn't happen. In this case, check for an empty list before the loop and set the varaible appropriately.

Otherwise, catch the exceptions (the one you expect might happen(*)), an throw a new more appropriate exception (with the original as the inner exception)

(*) Catch only the one's you expect might happen -- let the unexpected one bubble up to the user.

Answer 4

Code Complete suggests if you can handle the exception locally that you do so: "Don't use an exception to pass the buck". In the example given this would mean check for an empty value and handle this appropriately in the setter.