Search code examples
c#insert

object reference not set to an instance of an object >> insert query c#


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


Solution

  • 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.