debugcsharpModerate
Throwing exceptions when validation fails
Viewed 0 times
exceptionsfailsvalidationthrowingwhen
Problem
When I want to check the validity of an attendance being entered into the system, I perform following action.
AttendancePresenter Class
View
Appreciate if you could review this code and give your feedback. Specially what I want to know is about the exception thrown in presenter if the validation fails. Is it acceptable? Is this is a standard use of
AttendancePresenter Class
void _View_OnCheckValidity(object sender, EventArgs e)
{
//ExecuteAction performs exception handling in Base Class
this.ExecutAction(() =>
{
ValidateAttendance();
});
}
private void ValidateAttendance()
{
//DataService method returns true if the attendance is valid.
var validity = _DataService.CheckValidityOfAnAttendance(_View.EmployeeID, _View.Date, _View.ShiftType);
//Set the validity of the attendance in a View property.
//So that View can stop execution if validity is false.
_View.AttendanceValidity = validity;
//If validation fails, throw an exception
if (!validity)
{ throw new Exception("Invalid Attendance. Already there is a matching attendance for this employee"); }
}View
private void txtOutTime_KeyPress(object sender, KeyPressEventArgs e)
{
if (e.KeyChar == (char)Keys.Enter & !string .IsNullOrWhiteSpace(txtEmployeeID.Text) & txtOutTime.Text != DefaultText)
{
OnCheckValidity(sender, e);
if (this.AttendanceValidity)
{ OnEnterAttendance(sender, e); } //This line of code enters attendance into the DB
}
}Appreciate if you could review this code and give your feedback. Specially what I want to know is about the exception thrown in presenter if the validation fails. Is it acceptable? Is this is a standard use of
throw?Solution
You're throwing
You haven't shown the code where you
The problem with that, is that you don't know if the exception you're catching is due to a validation error, or if there's a division by zero, or if you ran out of memory, or if a database connection failed. With a custom exception type, you can do this:
And any exception thrown that is not a
That said, I wouldn't throw an exception for that. Exceptions should be for exceptional things. If you already know how you're going to handle it (show the error message in a message box?), why not just do that instead of throwing?
System.Exception. Don't do that. If you're going to have to throw an exception for a validation exception, throw a custom ValidationException exception.You haven't shown the code where you
catch and handle that exception, but it's going to have to look like this:try
{
// some code
}
catch (Exception exception)
{
// handle the [validation?] exception
}The problem with that, is that you don't know if the exception you're catching is due to a validation error, or if there's a division by zero, or if you ran out of memory, or if a database connection failed. With a custom exception type, you can do this:
try
{
// some code
}
catch (ValidationException exception)
{
// handle the validation exception
}And any exception thrown that is not a
ValidationException, will bubble up the stack.That said, I wouldn't throw an exception for that. Exceptions should be for exceptional things. If you already know how you're going to handle it (show the error message in a message box?), why not just do that instead of throwing?
Code Snippets
try
{
// some code
}
catch (Exception exception)
{
// handle the [validation?] exception
}try
{
// some code
}
catch (ValidationException exception)
{
// handle the validation exception
}Context
StackExchange Code Review Q#60176, answer score: 15
Revisions (0)
No revisions yet.