.Net Throwing exceptions from ToString?

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 no开发者_运维技巧t.

Any thoughts?

Code: http://pastebin.com/mLEkBAAz

Thanks.

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.

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.

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.

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).

If the object is in an invalid state you should throw an exception. If I see something like a NullReferenceException, well lets just say I won't be impressed.

My philosophy is that I am not smart enough to cover all cases so I throw exceptions to help myself and other programmers catch errors early. Also known as the Fail Fast philosophy.

Exceptions are part of the contract of object's ToString method and, as such, any exceptions thrown by derived class implementations should not differ to those thrown by the base class implementation, in my opinion. I don't think ToString throws any kind of exception in any of the Microsoft base BCL classes, so I'd stick to that convention.

As you said, if any other exception is thrown, it will be a surprise. Programmers don't like surprises.

ToString() is a method like any other, so if you want to report a problem in its execution you should throw an exception (as usual, don't use exceptions for expressing logic or returning results). If, as elysium mentions, debugging becomes problematic you can mark the ToString() method with the DebuggerStepThrough attribute.