Skip to main content
Code Review

Return to Answer

replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link

Constants in .NET should be named using PascalCase. See c-sharp-naming-convention-for-constants 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.

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.

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.

Source Link
Heslacher
  • 50.9k
  • 5
  • 83
  • 177

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<PaymentInfo> 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;
}
default

AltStyle によって変換されたページ (->オリジナル) /