There are multiple problems with this code, unfortunately none of the existing (now deleted) answers even mention, let alone help to fix.
- There's the SQL Injection risk marc_s warned about.
- All these commands must be inside a transaction.
- You are using
like
to get the student id, but you get the name from a list box - so why not have the id already as the value of the ListBoxItem?
- Before you can borrow a book, you must first make sure you have at least one copy left to borrow
- You are swallowing exceptions
- You are using a class-level variable for
SqlConnection
, while SqlConnection
is best used as a local variable inside a using statement.
- You are mixing code with display - this will be impossible or at least very hard to test. Testability is a part of Maintainability.
- Your variable naming is terrible. A variable name should be meaningful - so that when someone reads the code they can get the meaning of the variable just by reading it's name.
- The only thing that should be inside the
try
block is the actual database access. Exceptions and Try...catch
blocks are for things you can't test or control in your code.
All of that being said, here is a somewhat improved version of your code as a starting point. Please try to improve it further to fix all (or at least most of) the issues I've listed above:
private void button2_Click(object sender, EventArgs e)
{
if (dataGridView1.SelectedRows.Count != 0 && listBox1.SelectedIndex != -1)
{
using(var con = new SqlConnection(connectionString))
{
var trn = con.BeginTransaction();
var sqlGetStudentId = "select studentID from student where studentName = @studentName";
using(var cmd = new SqlCommand(sqlGetStudentId, conn))
{
cmd.Parameters.Add("@studentName", SqlDbType.VarChar).Value = listBox1.SelectedItem.ToString();
try
{
con.Open();
string studentId = cmd.ExecuteScalar()?.ToString();
if(!string.IsNullOrEmpty(studentId))
{
// Note: You must first check if there are books left to borrow!
cmd.CommandText = "update Book set quantity=quantity-1 where bookID = @BookId";
cmd.Parameters.Clear();
cmd.Parameters.Add("@BookId", SqlDbType.VarChar).Value = dataGridView1.CurrentRow.Cells[0].Value.ToString();
cmd.ExecuteNonQuery();
cmd.CommandText = "insert into Borrow (/* Alwasy specify columns list! */) values (@IAmGuessingBookId, @StudentId, GETDATE(), NULL)";
cmd.Parameters.Clear();
cmd.Parameters.Add("@IAmGuessingBookId", SqlDbType.VarChar).Value = dataGridView1.CurrentRow.Cells[0].Value.ToString();
cmd.Parameters.Add("@StudentId", SqlDbType.VarChar).Value = studentId;
cmd.ExecuteNonQuery();
trn.Commit();
}
}
catch(Exception ex)
{
// Do something with the exception!
// show an error description to the client,
// log for future analisys
trn.Rollback();
}
}
}
}
}
与恶龙缠斗过久,自身亦成为恶龙;凝视深渊过久,深渊将回以凝视…