.Net Throwing exceptions from ToString?

views:

138

answers:

+2 Q:

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

Any thoughts?

Code: http://pastebin.com/mLEkBAAz

Thanks.

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.

ChaosPandion 2010-08-02 23:41:25

Strictly speaking, an exception should be thrown before the object is even allowed to enter an invalid state. In fact, no object should ever be in an invalid state. If it is, there should be a flag property indicating that it is, or it should follow the state machine pattern.

Rei Miyasaka 2010-08-02 23:49:33

@Rei - In an ideal world.

ChaosPandion 2010-08-02 23:50:21

@Rei: That's what I'm trying to say.

devoured elysium 2010-08-02 23:51:01

@Chaos, There's a difference between an invalid state and an unusable state. Ensuring that an object is always in a usable state is often not possible, but ensuring that an object is always in a valid state is in fact very doable in the real world. After all, we're talking about computers here -- save for hardware failures or memory management bugs, we're good.

Rei Miyasaka 2010-08-02 23:56:34

@Rei - You don't have to tell me, 90% of the objects I write are immutable.

ChaosPandion 2010-08-03 00:02:59

+3 A:

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.

devoured elysium 2010-08-02 23:41:37

What if you have a mutable object with one valid state. If you are not in a valid state I would expect it to throw a detailed exception.

ChaosPandion 2010-08-02 23:43:28

In the .ToString() method? The idea of the .ToString() method is not to check if the object is in a good state. If it is in an invalid state, print that as string! That's what you'll want to see when you're debugging, for instance.

devoured elysium 2010-08-02 23:43:58

What if you are not using `ToString` for debugging? In visual studio you can catch all unhanded exceptions at the point they are thrown. What if you forget to check the result and all of the sudden at another point in your code the bug finally reveals itself.

ChaosPandion 2010-08-02 23:46:34

But why should the bug reveal itself when you're calling .ToString(), in the first place? The .ToString() method should be the last place you would be looking for an exception. You should look after object's invariants to check for invalid state and then raising exceptions, not waiting for a call to .ToString().

devoured elysium 2010-08-02 23:47:37

Mutable objects can have many invalid states. What if you forgot to set one property on the object?

ChaosPandion 2010-08-02 23:49:31

What I mean is, if the object is in an invalid state, it wasn't because of calling .ToString(). If it wasn't because of calling .ToString(), then you should launch the exception in the place where the invalid state was introduced, so you can easily find what went wrong.

devoured elysium 2010-08-02 23:50:04

My class is a representation of a QueryModel. To run the query though, it needs to be an xml string. So, I'm using ToString() to format my object in xml so that it can be run. However, if my object isn't in a valid state, I don't want the query to be run. So that is why I'm throwing the exception. I don't know how to better represent my object for ToString, other than the xml format. And I don't want the xml to be run if it's invalid, so thus the exception. Thoughts?

TehOne 2010-08-02 23:50:50

@TehOne - I think that is very reasonable. My philosophy is that I am not smart enough to cover all cases so I throw exceptions.

ChaosPandion 2010-08-02 23:52:07

Why don't you put a method called CheckIfIsInValidState() or something?

devoured elysium 2010-08-02 23:52:59

I agree that it's a bad idea to throw an exception in ToString(), if it is for making an Xml representation then implement a new method like ToXmlQuery() or something like that, and then you can throw an exception from there.

Jeff T 2010-08-02 23:54:27

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.

devoured elysium 2010-08-02 23:54:47

I've updated the post with a link to the code. I was thinking of making a ToXmlQuery() method or something, but then I don't know of how I'd want to make better use of ToString for an accurate representation of my object anyways, which is why I just used ToString in the first place. My object has no use other then being used as xml to submit to a webservice.

TehOne 2010-08-02 23:58:51

If it is only behaving like a value, it shouldn't have the responsability to throw exceptions in the first place. It should be the class that actually does something that has that responsability.

devoured elysium 2010-08-03 00:00:19

After looking at the code, I believe that the object should be refactored to not throw exceptions. However, I still stand by my opinion that it is valid to throw exceptions in `ToString` in a defensive manner. It is important to remember that there are always exceptions to hard and fast programming rules.

ChaosPandion 2010-08-03 00:00:33

I have to agree with @Jeff T and @devoured elysium. ToString should represent the object for informational purposes. This is a case of serialization, not display ("so it can be run"). You're not overriding the *implementation* of ToString(), you're overriding the *purpose* for ToString(). This is violating the spirit of the interface as you're essentially changing the return type from a (plain text) String to XML.

Jim Leonardo 2010-08-03 00:04:33

@Jim: You summed it up with the following sentence: "You're not overriding the implementation of ToString(), you're overriding the purpose for ToString()"

devoured elysium 2010-08-03 00:07:20

Thanks for all of the opinions guys. I have renamed my method and will just not be overriding ToString for now. As I stated in the question, I already felt like I was making a mistake, and I've now gotten enough opinions to make me correct it. Thanks again everyone.

TehOne 2010-08-03 00:07:56

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

tdammers 2010-08-02 23:50:33

+5 A:

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.

Rei Miyasaka 2010-08-02 23:51:45

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.

Mark Simpson 2010-08-02 23:55:41

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.

Edgar Sánchez 2010-08-02 23:58:25

The .ToString() method shouldn't do any *operation* on the object/system, imo. If it can't change the state of the object, why should it have the responsability of throwing exceptions? I think a lot of the confusion comes from StringBuilder's .ToString() method, which I find awkward. In one hand, it goes with the rest of the .NET convention of having .ToXXX's methods when you want to convert to some data type. On the other hand, .ToString() is taken as a special method in the framework, so I think maybe a StringBuilder.BuildString() could have been a more sugestive name and cause less havoc.

devoured elysium 2010-08-11 09:28:57

+2 A:

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.

Mark Rushakoff 2010-08-03 00:08:23

ansaurus

tags:

views:

answers:

.Net Throwing exceptions from ToString?

Edit:

related questions