patterncsharpModerate
Transaction handling for multiple SQL statements
Viewed 0 times
handlingsqlstatementsfortransactionmultiple
Problem
In this code I update two tables called
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
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
Your
As the value of
Refactoring
Before using the first item in the list, a check to see if the
won't be used, except for assigning the affetced rows of the update command. we should just remove it.
Using curly brackets
should be placed outside of the for loop. Ther is no need to assign the const to the
Result
Constants in .NET should be named using
PascalCase. See c-sharp-naming-convention-for-constantsYour
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.