.Net Throwing exceptions from ToString?

Question

Just curious if anyone has any opinions on throwing an exception in my overridden ToString implementation. My instincts tell me this might be bad practice, but I don't seem to be able to find anything supporting if this is bad or not.

Any thoughts?

Code: http://pastebin.com/mLEkBAAz

Thanks.

Answer 1

I wouldn't do it. I don't see any situation where it would be better to have a ToString() method throw an exception instead of, for instance, returning the Object.ToString() string's representation.

You generally use a lot of .ToString() calls for debugging purposes. Now, let's say you're debugging your code and trying to catch a bug. Not only is your buggy code throwing exceptions in random places, you also have to take care of the aditional problem of having object's string representations throwing exceptions.

Edit:

So, by what you're telling us, I probably wouldn't put the code you are puting in .ToString()'s. I'd find another method name, let's say, .GetXMLRepresentation() and I'd have also a .CheckIfIsInValidState(). Then, I'd make .GetXMLRepresentation() throw an exception if you tried to call it in an invalid state. But I'd like to use .ToString() for other kind of purposes.

Answer 2

An object should never be in an invalid state. The object should throw an exception as soon as the consumer tries to set an invalid state on it.

Your object could be in an unusable state (say it's a database connection object, and it hasn't connected yet), in which case it should have an IsConnected flag, or it should follow the state machine pattern so that the object's state is still valid in its own right.

And since your ToString() overload presumably doesn't take any arguments, it should never be the caller's fault that it would throw an exception.

Therefore, no, I can't think of any exceptions that ToString() would throw.

Edit: In the case of the pastebin code, the best way to go about doing this would be to consolidate the query parameters in a separate class -- call it SearchParameters or something.

Populate that first, and then pass that into a class that'll generate the SQL code. If when you pass the SearchParameters object into SearchQuery (probably through the constructor so that you can make it immutable) the parameters are invalid, you can throw an exception there.

That way if your SearchQuery object ever has more methods that rely on there being a valid search query, you won't need to repeat the validation code, and of course, ToString() will never throw an exception.

Answer 3

Gendarme (a static analysis tool) has a "Do Not Throw In Unexpected Location" rule that states:

Object.ToString - these are called by the debugger to display objects and are also often used with printf style debugging so they should not change the object's state and should not throw.

Hardly official by Microsoft, but this is a very good indicator that it would be poor practice to throw in a ToString method.

Answer 4

I'd say it's a bad idea. If the object is in an invalid state, there must be some point at which it got there - that's where you should throw, not when you convert it to string. IMO, any object should have a valid string representation at all times, even if it's the default fall-back (the object's class name).