HiveBrain v1.2.0
Get Started
← Back to all entries
debugcsharpModerate

Throwing exceptions when validation fails

Submitted by: @import:stackexchange-codereview··
0
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

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