Data Access Layer method with transaction handling

Posted on

Problem

I just want to know whether I’ve done transaction handling well. Also what about the other types of transactions available in .NET with regard to this code snippet?

Also are there any redundant things in this code?

    public void InsertAttendance(BindingList<IAttendance> attendanceList)
    {
        string selectStatement = @"IF NOT EXISTS (SELECT Attendance.Atten_ID FROM Attendance WHERE Atten_ID = @aid) BEGIN INSERT INTO attendance (EID,PID,in_time,out_time,shift) VALUES(@eid,@pid,@inT,@outT,@shift) END ";
        using (SqlConnection sqlConnection = new SqlConnection(db.ConnectionString))
        {
            sqlConnection.Open();
            using (SqlTransaction trans = sqlConnection.BeginTransaction())
            using (SqlCommand sqlCommand = new SqlCommand(selectStatement, sqlConnection, trans))
            {
                sqlCommand.Parameters.Add("@aid", SqlDbType.Int);
                sqlCommand.Parameters.Add("@pid", SqlDbType.Char);
                sqlCommand.Parameters.Add("@eid", SqlDbType.Int);
                sqlCommand.Parameters.Add("@inT", SqlDbType.DateTime);
                sqlCommand.Parameters.Add("@outT", SqlDbType.DateTime);
                sqlCommand.Parameters.Add("@shift", SqlDbType.Char);
                //sqlConnection.Open();

                try
                {
                    foreach (Attendance obj in attendanceList)
                    {
                        sqlCommand.Parameters["@aid"].Value = obj.AttendanceID;
                        sqlCommand.Parameters["@pid"].Value = obj.PointID;
                        sqlCommand.Parameters["@eid"].Value = obj.EmployeeID;
                        sqlCommand.Parameters["@inT"].Value = obj.InTime;
                        sqlCommand.Parameters["@outT"].Value = obj.OutTime;
                        sqlCommand.Parameters["@shift"].Value = obj.ShiftType;

                        sqlCommand.ExecuteNonQuery();
                    }
                    trans.Commit();
                }
                catch { trans.Rollback(); throw; }
            }
        }
    }

Solution

First and foremost, I would strongly suggest using an ORM (e.g., Entity Framework or nHibernate) rather than doing all this manual work to update the database. ORMs take most of the drudge work away and generally make things much easier.

That aside, there are a few things I would change:

  • As much as possible, utilize the ADO.NET interface methods instead of creating concrete classes directly
  • While a commit must be performed before leaving scope, rollback happens automatically, so the catch block is unnecessary
  • The method should not take in BindingList, both because it only requires IEnumerable and because DAL code shouldn’t have ties to view concerns
  • The SQL statement variable should be const
  • There is no need to prefix variables with sql – this smells like a form of Hungarian Notation

With that in mind, I would modify it to be more like the following:

public void InsertAttendance(IEnumerable<IAttendance> attendanceList)
{
    const string selectStatement = @"IF NOT EXISTS (SELECT Attendance.Atten_ID FROM Attendance WHERE Atten_ID = @aid) BEGIN INSERT INTO attendance (EID,PID,in_time,out_time,shift) VALUES(@eid,@pid,@inT,@outT,@shift) END ";
    using (var connection = new SqlConnection(db.ConnectionString))
    {
        connection.Open();
        using (var trans = connection.BeginTransaction())
        using (var command = connection.CreateCommand())
        {
            command.CommandText = selectStatement;
            var parameters = command.Parameters;

            parameters.Add("@aid", SqlDbType.Int);
            parameters.Add("@pid", SqlDbType.Char);
            parameters.Add("@eid", SqlDbType.Int);
            parameters.Add("@inT", SqlDbType.DateTime);
            parameters.Add("@outT", SqlDbType.DateTime);
            parameters.Add("@shift", SqlDbType.Char);

            foreach (var attendanceItem in attendanceList)
            {
                parameters["@aid"].Value = attendanceItem.AttendanceID;
                parameters["@pid"].Value = attendanceItem.PointID;
                parameters["@eid"].Value = attendanceItem.EmployeeID;
                parameters["@inT"].Value = attendanceItem.InTime;
                parameters["@outT"].Value = attendanceItem.OutTime;
                parameters["@shift"].Value = attendanceItem.ShiftType;

                command.ExecuteNonQuery();
            }
            trans.Commit();
        }
    }
}

Sadly, creating parameters is particularly painful using just IDbCommand, so I continued using the SqlParameterCollection members.

Additional questions/considerations:

  • Should PointID be char? An ID suggests a numeric type.
  • Should ShiftType be char? This name suggests an enum type, which I suppose you could choose to represent as a character in the database, but it could also be a numeric type.
  • The SQL statement no-ops if a record already exists, but the method gives no indication when this occurs. This should probably be re-considered, since it fails to provide any feedback to the caller that the insert did not occur.
  • What is db? It seems to be providing the connection string, but we only see the property reference.

Leave a Reply

Your email address will not be published. Required fields are marked *