Converting a string to an int
-
I found this gem of a code when I was reviewing some code I am supposed to enhance. And this kind of conversion code has been used all over the project. Note that variable and method names have been changed. Did the guy who wrote this code ever have any idea how much memory and processor cycles does throwing and catching exceptions entail?
int age;
string strAge = GetAge();try {
age = Convert.ToInt32(strAge);
} catch {
age = 0;
}Shameel wrote:
atrAge
Shameel wrote:
strAge
I think I see the problem!! :laugh:
Silence is golden... but duct tape is silver!! Booger Mobile - My bright green 1964 Ford Falcon - check out the blog here!! | If you feel generous - make a donation to Camp Quality!!
-
It was VERY common prior to .Net 2.0 which introduced the TryParse methods. There really wasn't a 'good' way to do this, so if the result of GetAge() was normally a valid number, then it wouldn't hurt performance much. The performance hit is only noticable if there is actually an exception thrown.
GibbleCH wrote:
The performance hit is only noticable if there is actually an exception thrown.
Exactly. And when I was debugging, it was indeed throwing exceptions at many places because the values were supposed to be coming from a missing configuration element.
-
Shameel wrote:
how much memory and processor cycles does throwing and catching exceptions entail?
Maybe, but perhaps you don't? Performance implications of Exceptions in .NET[^]
The idea was to convey that a mere check for numbers would have been enough in this case instead of relying on .NET's exception handling to assign default value for missing items (and unnecessarily create and dispose exception objects). And when I was debugging the code, it was throwing exception all over the places because the values were supposed to come from config elements which were missing. It's a bad practise even from a debugging perspective.
-
Shameel wrote:
atrAge
Shameel wrote:
strAge
I think I see the problem!! :laugh:
Silence is golden... but duct tape is silver!! Booger Mobile - My bright green 1964 Ford Falcon - check out the blog here!! | If you feel generous - make a donation to Camp Quality!!
-
Shameel wrote:
That was just a typo. I renamed the variables to save the identity of the culprit. :)
I know... it was funny though!!
Silence is golden... but duct tape is silver!! Booger Mobile - My bright green 1964 Ford Falcon - check out the blog here!! | If you feel generous - make a donation to Camp Quality!!
-
I found this gem of a code when I was reviewing some code I am supposed to enhance. And this kind of conversion code has been used all over the project. Note that variable and method names have been changed. Did the guy who wrote this code ever have any idea how much memory and processor cycles does throwing and catching exceptions entail?
int age;
string strAge = GetAge();try {
age = Convert.ToInt32(strAge);
} catch {
age = 0;
}This isn't bad, just outdated. Before the introduction of TryParse this is basically how you had to do it (though I would use int.Parse not Convert.ToInt32, but one calls the other I think). Although you could argue it should be in a single utility method, it's only a couple of lines so it's hardly worth it. A try block uses almost no resources, and the exception will only be thrown when the requested string isn't a valid number, so I doubt there's any noticeable performance impact from doing this. Annoying to deal with when you have a better solution available to you now, and you have debugging set to look at caught exceptions? Yeah, probably. But it's not really bad code. If you're complaining that the debugger stops on it, try adjusting your settings – otherwise you can use that as an argument against any catch blocks and I'm sure you can see how that becomes an absurd position to hold pretty quickly.
-
The idea was to convey that a mere check for numbers would have been enough in this case instead of relying on .NET's exception handling to assign default value for missing items (and unnecessarily create and dispose exception objects). And when I was debugging the code, it was throwing exception all over the places because the values were supposed to come from config elements which were missing. It's a bad practise even from a debugging perspective.
Shameel wrote:
a mere check for numbers
That's worse; it can still go wrong, a string of "9999999999999999999999999999999999999999" for instance. Using a built-in method is much simpler and if you have to protect against Exception, then do so, it doesn't cost anything.
-
This isn't bad, just outdated. Before the introduction of TryParse this is basically how you had to do it (though I would use int.Parse not Convert.ToInt32, but one calls the other I think). Although you could argue it should be in a single utility method, it's only a couple of lines so it's hardly worth it. A try block uses almost no resources, and the exception will only be thrown when the requested string isn't a valid number, so I doubt there's any noticeable performance impact from doing this. Annoying to deal with when you have a better solution available to you now, and you have debugging set to look at caught exceptions? Yeah, probably. But it's not really bad code. If you're complaining that the debugger stops on it, try adjusting your settings – otherwise you can use that as an argument against any catch blocks and I'm sure you can see how that becomes an absurd position to hold pretty quickly.
10!
BobJanova wrote:
one calls the other
Yes, Convert calls the specific class' method, Int32.Parse in this case -- that's why I tell people not to use Convert.
-
This isn't bad, just outdated. Before the introduction of TryParse this is basically how you had to do it (though I would use int.Parse not Convert.ToInt32, but one calls the other I think). Although you could argue it should be in a single utility method, it's only a couple of lines so it's hardly worth it. A try block uses almost no resources, and the exception will only be thrown when the requested string isn't a valid number, so I doubt there's any noticeable performance impact from doing this. Annoying to deal with when you have a better solution available to you now, and you have debugging set to look at caught exceptions? Yeah, probably. But it's not really bad code. If you're complaining that the debugger stops on it, try adjusting your settings – otherwise you can use that as an argument against any catch blocks and I'm sure you can see how that becomes an absurd position to hold pretty quickly.
BobJanova wrote:
you have debugging set to look at caught exceptions?
yes, I have to do that to handle some subtle exceptions that are caught and swallowed (empty catch blocks - now that deserves a separate post by itself).
BobJanova wrote:
try adjusting your settings
I hate to keep doing it every now and then to accomodate this kind of code. Unfortunately, for now, I have to live with it since I cannot change the behavior of existing "working code".
-
BobJanova wrote:
you have debugging set to look at caught exceptions?
yes, I have to do that to handle some subtle exceptions that are caught and swallowed (empty catch blocks - now that deserves a separate post by itself).
BobJanova wrote:
try adjusting your settings
I hate to keep doing it every now and then to accomodate this kind of code. Unfortunately, for now, I have to live with it since I cannot change the behavior of existing "working code".
But my point is that 'this kind of code' is basically any code that uses exceptions. Swallowed exceptions generally deserve a post here, yes. This code handles the exception to put the variable into a valid state so it's not in the same category. Perhaps better than changing your settings would be to make the config file valid.
-
Shameel wrote:
how much memory and processor cycles does throwing and catching exceptions entail?
Maybe, but perhaps you don't? Performance implications of Exceptions in .NET[^]
Sounds like you didn't RTFA: "Only primitive string processing was affected by raised exceptions" I'd describe a conversion from an string to an int as fitting that description. Generally, however, I agree - people are too fixated with the costs of exceptions. If there's any I/O going on at all, its negligible.