As you can see in the MSDN StringValidator documentation, the Validate
method returns void
.
If validation doesn't succeed the Validate
method throws ArgumentException
.
I thought that "You only throw an exception when something exceptional happens".
Surely a validator that failed to validate isn't exceptional..
Why not return bool? What am I missing here?
Is this a "style issue" (i.e. if it was returning bool, it would still be correct, but just different style)?
Note: Method CanValidate may have something to do with it, but I still see no reason for this behavior.
I agree that it should return a boolean, failure to validate is not exceptional in many real world application.
You are correct when you say:
Between the two, the user of this code has the ability to receive both a non-exceptional response to a check for validation, and also to trigger an exception. That both of these options exists suggests that the designers of the class see at least some reasonable cases for each approach. Assuming their design isn't entirely boneheaded (or even without that assumption, since it applies elsewhere), lets consider cases where both make sense.
Let's consider a method
static MySettings ReadSettings(string filePath)
. This method will obtain some sort of meaningful object from the contents of a file. Let's also imagine that we do every bit of this "close to the metal", without depending on the file-operation classes in the BCL; not because that's a good idea, but because it brings all issues of exception handling right into the realms of what we have to deal with (in reality we can lump several of the exceptions we're going to consider together by allowing the BCL file-related methods to either return us a usable stream or else throw an exception for several possibilities, but we want to consider those possibilities here).Possible things that can go wrong are:
filePath
is not valid.Notably, 7 and 8 are not cases we are explicitly coding for; by their nature they are things we haven't thought to code for.
Now. Some of this is impossible to completely avoid; e.g. even if we check for existence of a file, that introduces a race condition (it can be deleted afterwards) and doesn't preclude disk-error part of the way through reading it.
Let's consider what using code will do in this case. It's going to be pretty rare, so possibly using code will fail to check for it (you may argue that's imperfect, but assuming perfection in calling code is poor design for your code). It may be that there is a clear action that could be taken in this case (e.g. if the calling code is close to the UI then it can respond with a message to the user), but it may be that there isn't, which forces it to in turn respond to the code that called it with a signal that the operation failed, along with possibly doing so for other things that can go wrong at that level (simply returning a boolean of success is of little value in knowing why something failed, and what should be done with it). So, we need a mechanism for signalling the failure that can go all the way up the call stack until we get to something that can either:
When writing 'ReadSettings' we have not just no idea as to which of these will be appropriate, but more importantly no idea of how far the code that does this will be from our code, and no idea of what else could go wrong in the set of operations of which ours are only one part. The description of what we need above (ability to climb up the call-stack, descriptiveness, ability to be mixed with dealing of similar signals from other methods) is a reasonably good description of precisely what the exception mechanism gives us. So when any of the possibilities above happen we throw an exception, apart from the last two possibilities (that we haven't planned for), though ideally we can make it so that this also throws an exception.
First approach at planning this, our set of procedures are:
MySettings
object (exception if failure).MySettings
object is reasonably valid (exception if failure).MySettings
object.There are problems here. First, our attempt to obtain the file handle depends on failing if the handle is invalid. This works but may be more expensive that testing first (depending on how much I/O work has gone on before the failure), and much more importantly can introduce the possibility of our blindly doing something stupid at the I/O level which could introduce a security issue (what sort of issue? I don't know, and if I deal with validation first, I don't even have to know). Hence we introduce step 0, which is validating the filePath. We could make this a check followed by an explicit exception-throw right in our
ReadSettings
, but the same rules for validation probably exist elsewhere (e.g.WriteSettings
), so to avoid duplicated effort and (more importantly for correctness) to centralise the validation logic, we might have avoid CheckValidSettingsPath(string filePath)
method that throws anArgumentException
if the path isn't valid.We can also now get away from the madness of doing all the file operations at a low level, by using BCL classes and depending on their failing for many of the possible failure conditions. One bonus of this is that this may well take out a few (or even a large swathe) of "cases we haven't thought of", since the BCL file operation classes may have thought of them even if we didn't!
We can also have some invariants in the MySettings class, so we can offload some of the responsibility for checking correctness to there. We now have:
MySettings
object (exception if failure -MySettings
does if for us at least some of the time).MySettings
beyond those inherent to allMySettings
objects (exception if failure).MySettings
object.We've reduced the complexity. An issue remains as to whether we should just allow these exceptions to pass through, eat them and throw our own, or wrap them in a new one of our own. As a rule I would allow most to pass through, but in public methods of public classes wrap them in a new one (so on the one hand the user of the code sees
ReadSettings
failing rather than a privateCheckValidSettingsPath
she's never heard of, but on the other, the details of what went wrong at the lower level is still available [there may sometimes be security reasons for hiding this]); eating and throwing a "fresh" exception of ones own (noinnerException
) works well only when it is sure to be able to completely explain what went wrong.So, this is a case comparable to
StringValidator.Validate
. Indeed, it could well be used as part of ourCheckValidSettingsPath
method.Let's now consider the case where we are writing code that calls
ReadSettings
. Obviously we have a string that contains a filePath:Possibly, (because of the nature of how we got that string) the chances of it being invalid are either nil, or could only happen if something has gone seriously wrong elsewhere in the application. In this case there is no point doing any validation here; we want to just try and let the exception happen if it will.
Possibly, there's not much we can do in the case of a failure, except for have that failure happen anyway. Again, we just try and let the exception happen if it will.
Possibly, we want to know we've a reasonable chance of
ReadSettings
succeed, before we do several steps of whichReadSettings
is not the most immediate (i.e. check it is likely to work, then do several other things, and then read the settings). Here an ability to check validity first is useful, and we then throw an exception when it happens. (There are several reasons why, if you are going to fail, it's good to fail early, which I won't go into).Finally, possibly we are close to the UI level and want to present a user-message ASAP in the case of the user presenting us with an obviously invalid input.
In these last two cases we can benefit from a method that operates much like
CheckValidSettingsPath
but which returns a boolean rather than throws an exception. In the first few possibilities, having an invalid path is either truly exceptional, or as close to it as makes no practical difference. In the last two possibilities, having an invalid path is not exceptional, and should ideally not be treated as such.Note that there is still the possibility that after we've checked the path is valid, the
ReadSettings
still goes wrong, so we will combine both the explicit-check-with-boolean approach and the implicit-check-during-operation approach that throws an exception.It is for cases like the last two that
StringValidator.CanValidate
exists, and again it could well be used of part of aIsValidSettingsPath
method.There could be cases where an application is architected in such a way that it would be an 'exceptional event' for the validation to fail at the point where this code is being called; where the calling code simply wants to do one, last double-check quickly and just "assume it's all OK" - and yet still be able to be notified if something unexpected has happened to cause the value to fail validation.
I would say those cases might be uncommon, but...
see ArgumentException Class
Can you use bool to replace the info that ArgumentExeption may contain? And based on this page,
So if the argument "Object" in
StringValidator.Validate(Object)
is not valid, what should be the best choice? Return a whole variety of objects or just throw ArgumentException?StringValidator
, along with any type derived fromConfigurationValidatorBase
are intended to be used with the .NET configuration system (see Jon Rista's article for a great overview of its capabilities). A simple use case is as follows:In this case, the
StringValidatorAttribute
instantiates aStringValidator
internally, which derives fromConfigurationValidatorBase
. Consequently, any type ofConfigurationValidatorBase
may be used with a configuration property (given that the types of the property and what validated match), and so I believe the configuration system needs to interact with this abstraction.If the validation method of
ConfigurationValidatorBase
always returned a Boolean value, how is it possible to determine what the specific validation error is, for any property, for any type of validation? I'm sure the type could have been designed to return both a string and Boolean, but then it would be difficult to get additional information from the error as the string would need to be parsed (i.e., the value causing the length validation error).Using an exception seems to be a good mitigating solution without introducing additional complexity to the system.