views:

162

answers:

5

I'm using ASP.NET CompareValidator controls to do data type checks. Should I trust these controls enough to directly parse their values or should I use TryParse?

Example:

<asp:TextBox ID="uxVolume" runat="server" />
<asp:CompareValidator ID="uxVolumeDataTypeValidator" runat="server" 
    ControlToValidate="uxVolume" ErrorMessage="Volume must be a number." 
    Type="Double" Operator="DataTypeCheck" Text="*" Display="Dynamic" />

in the code behind page should I Parse:

var volume = double.Parse(uxVolume.Text);
// do something

or TryParse:

double volume;
if (double.TryParse(uxVolume.Text, out volume))
{
    // do something
}
A: 

In my experience trusting the validator is safe. However, you can't go wrong by doing a TryParse in your codehind.

ichiban
+1  A: 

Personally, I wouldn't bother. If the validator fails, that means something real hokey is going on, so exceptioning out on double.Parse() probably isn't entirely bad at that point.

I've used them a few thousand times and have not had a problem . . .

Wyatt Barnett
+1  A: 

In that case you are expecting the validator to run correctly so I would not use the tryparse. That way if someone changed your validator then an exception would be thrown, instead of silently failing if you had used the tryparse

Jake Scott
A: 

I would use TryParse. The validator should work. I haven't seen and instance where it doesn't. That being said, you code should always try and protect itself from bad values. The code now may have the validator, but it might not in the future, so you can't rely on it being there, and just because the validator should work, doesn't mean that it always will (you could accidently move the code to parse the variable before the validator fires on the server side). As simple as it is to replace Parse with TryParse, there is no reason not to use TryParse and protect yourself from a potential issue of an unvalidated input item.

In response to Jakes comment. He's write you'll know if the validation doesn't work by throwing an error, and in that case it will have to be in a try catch block. So in this instance you are really doing the same thing as the TryParse statement except that you have to write extra code to handle the potential exception being thrown and the throwing exceptions is slower than checking a bool for success.

Kevin
+2  A: 

Yes but if someone changes/removes your validator, then you really do want the exception to be thrown so that you know there is problem with the application. Fail early fail fast (or something like that). Also you do not have to add extra try catch block because this should be an exception and be caught by your global error handler. In web config customerErrors code=500 or something like that.

Jake Scott