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

Transaction handling for multiple SQL statements

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
handlingsqlstatementsfortransactionmultiple

Problem

In this code I update two tables called Payment and SalaryTrans. First I insert records (Salary payments) to Payment table and then I update SalaryTrans table. When a record is inserted to Payment table, ID is auto generated (auto increment column) and then that ID should be updated in SalaryTrans table (for all matching records).

The following code compiles and runs giving the desired output, but I just want to know whether the transactions are handled properly. Also, are there any other flows in this code?

Could you please review and give your feedback?

```
public int InsertPaymentDetails(List list, int totalRecords, decimal totalAmount )
{
const string selectSatement = @"INSERT INTO Payment (payment_type, reference, payment_date, total_records, total_amount)
VALUES(@type, @reference, @date, @totalRecords, @totalAmount ) ";

const string updateStatement = @"UPDATE SalaryTrans SET payment_id = (SELECT TOP 1 id FROM Payment ORDER BY Payment.id DESC)
WHERE SalaryTrans.employee_id = @employeeID AND SalaryTrans.reference = @reference ";

//An item is refered in the list just to get the PaymentType, PaymentDate etc. as these information are common for all items in the list
var first = 0;
var payInfo = list[first];

//To get the affected rows following variables are declared
int result= 0;
int affectedRecords = 0;

using (SqlConnection sqlConnection = new SqlConnection(db.ConnectionString))
{
sqlConnection.Open();
using (SqlTransaction sqlTrans = sqlConnection.BeginTransaction ())
using (SqlCommand sqlCommand = new SqlCommand(selectSatement, sqlConnection,sqlTrans))
{
sqlCommand.Parameters.Add("@type", SqlDbType.VarChar).Value = payInfo.PaymentType;
sqlCommand.Parameters.Add("@date", SqlDbType.Date).Value = payInfo.PaymentDate;
sqlCommand.Parameters.Add("@reference", SqlDbType.VarChar).Value = payInfo.Reference;
sqlComm

Solution

Naming

Constants in .NET should be named using PascalCase. See c-sharp-naming-convention-for-constants

Your const string selectSatement = @"INSERT INTO Payment..." should be better named InsertStatement to also reflect that it is an INSERT and not a SELECT.

var first = 0;
var payInfo = list[first];


As the value of var first won't be changed, you should change it to a const with a meaningful name.

Refactoring

Before using the first item in the list, a check to see if the list is null or empty should take place.

const int NoRowsAffected = 0;
if (list == null || !list.Any()) { return NoRowsAffected; }


int result= 0;


won't be used, except for assigning the affetced rows of the update command. we should just remove it.

if (result == +1)
    affectedRecords = 1;


Using curly brackets {} should be a have to everytime, at least if written on separate lines.

sqlCommand.CommandText = updateStatement;


should be placed outside of the for loop. Ther is no need to assign the const to the CommandText property for each iteration.

Result

public int InsertPaymentDetails(List paymentInfos, int totalRecords, decimal totalAmount)
{
    const string InsertStatement = @"
        INSERT INTO Payment (payment_type, reference, payment_date, total_records, total_amount) 
        VALUES(@type, @reference, @date, @totalRecords, @totalAmount )
        ";

    const string UpdateStatement = @"
        UPDATE SalaryTrans 
        SET payment_id = (SELECT TOP 1 id FROM Payment ORDER BY Payment.id DESC) 
        WHERE SalaryTrans.employee_id = @employeeID 
        AND SalaryTrans.reference = @reference 
        ";

    const int NoRowsAffected = 0;
    const int FirstListIndex = 0;

    if (paymentInfos == null || !paymentInfos.Any()) { return NoRowsAffected; }

    var payInfo = paymentInfos[FirstListIndex];

    //To get the affected rows following variables are declared

    int affectedRecords = NoRowsAffected;

    using (SqlConnection sqlConnection = new SqlConnection(db.ConnectionString))
    {
        sqlConnection.Open();
        using (SqlTransaction sqlTrans = sqlConnection.BeginTransaction())
        using (SqlCommand sqlCommand = new SqlCommand(InsertStatement, sqlConnection, sqlTrans))
        {
            sqlCommand.Parameters.Add("@type", SqlDbType.VarChar).Value = payInfo.PaymentType;
            sqlCommand.Parameters.Add("@date", SqlDbType.Date).Value = payInfo.PaymentDate;
            sqlCommand.Parameters.Add("@reference", SqlDbType.VarChar).Value = payInfo.Reference;
            sqlCommand.Parameters.Add("@totalRecords", SqlDbType.Int).Value = totalRecords;
            sqlCommand.Parameters.Add("@totalAmount", SqlDbType.Decimal).Value = totalAmount;

            sqlCommand.ExecuteNonQuery();

            SqlParameter paramEmployeeID = new SqlParameter("@employeeID", SqlDbType.Int);
            sqlCommand.Parameters.Add(paramEmployeeID);
            sqlCommand.CommandText = UpdateStatement;

            foreach (PaymentInfo paymentInfo in paymentInfos)
            {
                paramEmployeeID.Value = paymentInfo.EmployeeID;
                //If at least one recored was inserted then the recordsAffected should be +1
                if (sqlCommand.ExecuteNonQuery() == 1) { affectedRecords = 1; }
            }
            sqlTrans.Commit();
        }
    }
    return affectedRecords;
}

Code Snippets

var first = 0;
var payInfo = list[first];
const int NoRowsAffected = 0;
if (list == null || !list.Any()) { return NoRowsAffected; }
int result= 0;
if (result == +1)
    affectedRecords = 1;
sqlCommand.CommandText = updateStatement;

Context

StackExchange Code Review Q#59863, answer score: 12

Revisions (0)

No revisions yet.