Follow Up post: Proper handling of exceptions in MVP

This is a follow up post of my early question "Proper handling of exceptions in MVP"

So based on the answers I re edited the post and now my exception handling code looks like this...

DAL

```
public bool InsertAccount(IBankAccount ba)
{
string insertStatement = @"IF NOT EXISTS (SELECT ac_no FROM BankAccount WHERE ac_no=@ac_no) BEGIN INSERT INTO BankAccount (ac_no, emp_id, ac_name, bank_name, ac_type," +
"ent_date, ent_by, remarks, file_no, status) VALUES (@ac_no, @emp_id, @ac_name, @bank_name, @ac_type, @ent_date, @ent_by, @remarks, @file_no, @status) END";

using (SqlConnection sqlConnection = new SqlConnection(db.ConnectionString))
{
using (SqlCommand sqlCommand = new SqlCommand(selectStatement, sqlConnection))
{
try
{
sqlConnection.Open();
sqlCommand.Parameters.Add("@ac_no", SqlDbType.Char).Value = ba.AccountNumber;
sqlCommand.Parameters.Add("@emp_id", SqlDbType.Int).Value = ba.EmployeeID;
sqlCommand.Parameters.Add("@ac_name", SqlDbType.VarChar).Value = ba.AccountName;
sqlCommand.Parameters.Add("@bank_name", SqlDbType.VarChar).Value = ba.BankName;
sqlCommand.Parameters.Add("@ac_type", SqlDbType.VarChar).Value = ba.AccountType;
sqlCommand.Parameters.Add("@ent_date", SqlDbType.DateTime).Value = ba.EnteredDate;
sqlCommand.Parameters.Add("@ent_by", SqlDbType.VarChar).Value = "ABCD";
sqlCommand.Parameters.Add("@remarks", SqlDbType.VarChar).Value = ba.Remarks;
sqlCommand.Parameters.Add("@file_no", SqlDbType.Int).Value = ba.FileNo;
sqlCommand.Parameters.Add("@status", SqlDbType.Bit).Value = ba.Active;

sqlCommand.ExecuteNonQuery();
return true;
}
catch (SqlException)
{

I can follow the reasoning behind the catch and rethrow even if I might not fully agree with it - without a fuller understanding of what you would like to achieve I will not try to offer an alternative.

The only comments I can see at the moment are:

HouseKeeping

InsertAccount() still has a return value even though it is never used. It can/should be removed.

Don't Repeat Yourself (DRY)

If you have a lot of calls to database, you do not want to have to repeat the exception handling (logging) code in each of them.

At the simplest level you can have a member in the data service that takes in an Action, executes it and handles any exceptions

private void ExecutNonQuery(Action nonQueryAction) {
    try {
       nonQueryAction();
    }
    catch (SqlException) {
        throw new Exception("DataBase related Exception occurred");
    }
    catch (Exception) {
        throw new Exception("Exception occurred");
    }
}

public void InsertAccount(IBankAccount ba)    {
  ExecuteNonQuery(()=> {
         string insertStatement = @"IF NOT EXISTS (SELECT ac_no FROM BankAccount WHERE ac_no=@ac_no) BEGIN INSERT INTO BankAccount (ac_no, emp_id, ac_name, bank_name, ac_type," +
                              "ent_date, ent_by, remarks, file_no, status) VALUES (@ac_no, @emp_id, @ac_name, @bank_name, @ac_type, @ent_date, @ent_by, @remarks, @file_no, @status) END";

        //...
  });
}

As I said, this is a minimal version. The next level is to refactor out the common code for connection and command creation (a bit more work but it can be done).

MessageService

As I said, I favour the MessageService approach but with one caveat. It is not obvious from the code if MessageService.ShowErrorMessage() is a static method on a static class (equivalent to the call the MessageBox) or if MessageService is a property of the class containing SaveBankAccount(). If a static, you still have the problem with the unit tests as a MessageBox is still shown each time.

StackExchange Code Review Q#54186, answer score: 2