I have this method which inserts data to two different tables in my database from textboxes and Datagridview , The first insert query executing normally but the second one throws an error
Object reference not set to an instance of an object
Here is my method:
private void InsertAll()
{
DialogResult Result = MessageBox.Show("Do you want to save all ? ", "Warnning", MessageBoxButtons.OKCancel, MessageBoxIcon.Warning);
if (Result == DialogResult.OK)
{
using (SqlCommand cmd = new SqlCommand())
{
try
{
cmd.Connection = Cn;
cmd.CommandType = CommandType.Text;
Cn.Open();
// Insert the header first Due to one-to-many relationship
cmd.CommandText = @" INSERT INTO DocDtls ( DocNum, zDate, Warehouse, Orientation,TransType,UserName )
VALUES (@prm1, @prm2, @prm3, @prm4, @prm5, @prm6);";
cmd.Parameters.AddWithValue("@prm1", txtDocNum.Text);
cmd.Parameters.AddWithValue("@prm2", txtDate.Value.ToString("yyyy-MM-dd"));
cmd.Parameters.AddWithValue("@prm3", cmbWh.Text);
cmd.Parameters.AddWithValue("@prm4", txtOrient.Text);
cmd.Parameters.AddWithValue("@prm5", txtTransType.Text);
cmd.Parameters.AddWithValue("@prm6", txtUser.Text);
cmd.ExecuteNonQuery();
// Insert the Details >> where the problem start to occur
if (txtTransType.Text == "Release")
{
cmd2.Connection = Cn;
cmd2.CommandType = CommandType.Text;
// Cn.Open();
for (int i = 0; i < DGV1.Rows.Count; i )
{
cmd2.CommandText = @"INSERT INTO Transactions ( DocNum, Code, QtyIn, QtyOut, BalanceAfter, Remarks, Unit)
VALUES (@prm1, @prm2, @prm3, @prm4, @prm5, @prm6,@prm7);";
cmd2.Parameters.AddWithValue("@prm1", txtDocNum.Text);
cmd2.Parameters.AddWithValue("@prm2", DGV1.Rows[i].Cells["Code"].Value);
cmd2.Parameters.AddWithValue("@prm3", 0);
cmd2.Parameters.AddWithValue("@prm4", DGV1.Rows[i].Cells["Qty"].Value);
cmd2.Parameters.AddWithValue("@prm5", DGV1.Rows[i].Cells["BalanceAfter"].Value);
cmd2.Parameters.AddWithValue("@prm6", DGV1.Rows[i].Cells["Remarks"].Value);
cmd2.Parameters.AddWithValue("@prm7", DGV1.Rows[i].Cells["Unit"].Value);
cmd2.ExecuteNonQuery();
MessageBox.Show("Registered", "Done", MessageBoxButtons.OK, MessageBoxIcon.Information);
}
}
else
{
cmd2.Connection = Cn;
cmd2.CommandType = CommandType.Text;
//Cn.Open();
for (int i = 0; i < DGV1.Rows.Count; i )
{
cmd2.CommandText = @"INSERT INTO Transactions ( DocNum, Code, QtyIn, QtyOut, BalanceAfter, Remarks, Unit )
VALUES (@prm1, @prm2, @prm3, @prm4, @prm5, @prm6,@prm7);";
cmd2.Parameters.AddWithValue("@prm1", txtDocNum.Text);
cmd2.Parameters.AddWithValue("@prm2", DGV1.Rows[i].Cells["Code"].Value);
cmd2.Parameters.AddWithValue("@prm3", DGV1.Rows[i].Cells["Qty"].Value);
cmd2.Parameters.AddWithValue("@prm4", 0);
cmd2.Parameters.AddWithValue("@prm5", DGV1.Rows[i].Cells["BalanceAfter"].Value);
cmd2.Parameters.AddWithValue("@prm6", DGV1.Rows[i].Cells["Remarks"].Value);
cmd2.Parameters.AddWithValue("@prm7", DGV1.Rows[i].Cells["Unit"].Value);
cmd2.ExecuteNonQuery();
MessageBox.Show("Registered", "Done", MessageBoxButtons.OK, MessageBoxIcon.Information);
}
}
}
catch (Exception e)
{
MessageBox.Show(e.Message.ToString(), "Error Message",MessageBoxButtons.OK,MessageBoxIcon.Error);
}
Cn.Close();
}
}
}
I suspected that the connection might be closed in some point so I added opening connection again after the first insert but the problem still happening.
What's wrong with my code?
Thanks in advance
CodePudding user response：
Try this. There's a lot of code, so likely a typo or other issue or two still here. Pay attention to the comments:
private void InsertAll()
{
using (var cn = new SqlConnection(Cn.ConnectionString)) // Using block around the connection
using (var cmd = new SqlCommand("", cn))
{
//removed the try/catch. Instead, wrap the try/catch around this method call
cmd.CommandText = @" INSERT INTO DocDtls ( DocNum, zDate, Warehouse, Orientation,TransType,UserName )
Values (@prm1, @prm2, @prm3, @prm4, @prm5, @prm6);";
//AddWithValue() can be dangerously slow, but usually still safe for single simple INSERT
cmd.Parameters.AddWithValue("@prm1", txtDocNum.Text);
cmd.Parameters.AddWithValue("@prm2", txtDate.Value); //If you're converting a DateTime to a string for use in SQL, you're doing something VERY WRONG
cmd.Parameters.AddWithValue("@prm3", cmbWh.Text);
cmd.Parameters.AddWithValue("@prm4", txtOrient.Text);
cmd.Parameters.AddWithValue("@prm5", txtTransType.Text);
cmd.Parameters.AddWithValue("@prm6", txtUser.Text);
cn.Open(); //Wait until last possible moment to call cn.Open()
cmd.ExecuteNonQuery();
// Normally don't re-use the same connection, but
// within a single operation context like this it's okay
// not only for the connection, but for the command, too.
cmd.CommandText = @" INSERT INTO Transactions ( DocNum, Code, QtyIn, QtyOut, BalanceAfter, Remarks, Unit )
Values (@DocNum, @Code, @QtyIn, @QtyOut, @BalanceAfter, @Remarks,@Unit);";
cmd.Parameters.Clear(); //Missing this line is why you had to create cmd2 before.
// Use exact types from the database here. Don't let ADO.Net try to infer these.
cmd.Parameters.Add("@DocNum", SqlDbType.VarChar, 10).Value = txtDocNum.Text;
cmd.Parameters.Add("@Code", SqlDbType.Char, 7);
// the "= 0" at the end of the next two lines are important.
cmd.Parameters.Add("@QtyIn", SqlDbType.Int).Value = 0;
cmd.Parameters.Add("@QtyOut", SqlDbType.Int).Value = 0;
cmd.Parameters.Add("@BalanceAfter", SqlDbType.Decimal);
cmd.Parameters.Add("@Remarks", SqlDbType.NVarChar, 1000);
cmd.Parameters.Add("@Unit", SqlDbType.NVarChar, 10);
// We can also set the Qty switch outside the loop
var qtyKey = (txtTransType.Text == "Release")?"@QtyOut":"@QtyIn";
for (int i = 0; i < DGV1.Rows.Count; i )
{
// Only change parameter values inside the loop
cmd.Parameters["@Code"] = DGV1.Rows[i].Cells["Code"].Value;
cmd.Parameters[qtyKey].Value = DGV1.Rows[i].Cells["Qty"].Value;
cmd.Parameters["@BalanceAfter"] = DGV1.Rows[i].Cells["BalanceAfter"].Value;
cmd.Parameters["@Remarks"] = DGV1.Rows[i].Cells["Remarks"].Value;
cmd.Parameters["@Unit"] = DGV1.Rows[i].Cells["Unit"].Value;
cmd.ExecuteNonQuery();
}
// No need to call cn.Close(). Using blocks take care of it.
}
}
Note I removed the initial prompt/check, the try/catch, and the final complete message. Those things belong with the caller, like this:
DialogResult Result = MessageBox.Show("Do you want to save all ? ", "Warning", MessageBoxButtons.OKCancel, MessageBoxIcon.Warning);
if (Result == DialogResult.OK)
{
try
{
InsertAll()
MessageBox.Show("Registered", "Done", MessageBoxButtons.OK, MessageBoxIcon.Information);
}
catch(Exception e)
{
MessageBox.Show(e.Message.ToString(), "Error Message",MessageBoxButtons.OK,MessageBoxIcon.Error);
}
}
Eventually you would also move the method (and other database access) be a static member of a separate class, where the required information is passed as arguments.