Welcome to OStack Knowledge Sharing Community for programmer and developer-Open, Learning and Share
Welcome To Ask or Share your Answers For Others

Categories

0 votes
449 views
in Technique[技术] by (71.8m points)

asp.net - C# SqlCommands are not working

I have a problem that the code isn't working, I'm trying to populate the database that saved in localhost on SQL Server but the code is doing nothing :(

private void button2_Click(object sender, EventArgs e)
{
    try
    {
        conn.Open();

        if (dataGridView1.SelectedRows.Count != 0 && listBox1.SelectedIndex != -1)
        {
            string id = "select studentID from student where studentName like '%" + listBox1.SelectedItem.ToString() + "%'";

            SqlCommand a = new SqlCommand(id, conn);

            a.ExecuteNonQuery();

            SqlDataReader reader = a.ExecuteReader();

            string s = reader.GetString(0);

            string q = "insert into Borrow values (" + dataGridView1.CurrentRow.Cells[0].Value.ToString()
                + ", " + s + " , '" + DateTime.Now + "' , Null)";

            SqlCommand cmd = new SqlCommand(q, conn);

            cmd.ExecuteNonQuery();
            MessageBox.Show("Book is now Borrowed");

            string tmp = "update Book set quantity=quantity-1 where bookID " + dataGridView1.CurrentRow.Cells[0].Value.ToString();
            SqlCommand tm = new SqlCommand(tmp, conn);
        }
    }
    catch
    {
    }
    finally
    {
        conn.Close();
    }
}
See Question&Answers more detail:os

与恶龙缠斗过久,自身亦成为恶龙;凝视深渊过久,深渊将回以凝视…
Welcome To Ask or Share your Answers For Others

1 Answer

0 votes
by (71.8m points)

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();
                }
            }
        }

    }
}

与恶龙缠斗过久,自身亦成为恶龙;凝视深渊过久,深渊将回以凝视…
Welcome to OStack Knowledge Sharing Community for programmer and developer-Open, Learning and Share
Click Here to Ask a Question

...