C# Datagridview to Access database

Question

I feel like there would be a better way to this. Essentially, I am transporting the data from an excel file to a DGV, and then from a DGV to an Access database. Should I cut the middle man or is this fine the way it is? The problem here is that my cells structure has to match access very closely to work.

private void btnImport_Click(object sender, EventArgs e)
    {
        DialogResult dr = MessageBox.Show(
            "Warning: when importing data into the Access database, ensure that the field columns match Access's fields or the file may become corrupt. Do you still wish to proceed?","Import caution",
            MessageBoxButtons.OKCancel, MessageBoxIcon.Warning);
        if (dr == DialogResult.OK)
        {
            try
            {

                using (OleDbConnection conn = new OleDbConnection(Con))
                {
                    using (OleDbCommand cmd = new OleDbCommand())
                    {
                        cmd.Connection = conn;
                        conn.Open();
                        cmd.CommandText =
                                "Insert INTO ACTB (FirstName, LastName, GrossIncome, LessTNT, TaxableIncomeCE, TaxableIncomePE, GrossTaxableIncome, LessTE, LessPPH, NetTax, TaxDue, HeldTaxCE, HeldTaxPE, TotalTax, PersonID) " +
                                "VALUES(@First, @Last, @Gross, @LessTNT, @TCI, @ADDTI, @GTI, @LessTE, @LessPPH, @LessNTI, @TD, @TWCE, @TWPE, @TATW, @PersonID)";
                        for (int s = 0; s < DGVExcel.Rows.Count - 1; s++)
                        {
                            cmd.Parameters.Clear();
                            //cmd.Parameters.AddWithValue("@ID", Convert.ToInt32(txtID.Text));
                            cmd.Parameters.AddWithValue("@First", DGVExcel.Rows[s].Cells[0].Value);
                            cmd.Parameters.AddWithValue("@Last", DGVExcel.Rows[s].Cells[1].Value);
                            cmd.Parameters.AddWithValue("@Gross", Convert.ToDouble(DGVExcel.Rows[s].Cells[2].Value));
                            cmd.Parameters.AddWithValue("@LessTNT",
                                Convert.ToDouble(DGVExcel.Rows[s].Cells[3].Value));
                            cmd.Parameters.AddWithValue("@TCI", Convert.ToDouble(DGVExcel.Rows[s].Cells[4].Value));
                            cmd.Parameters.AddWithValue("@ADDTI", Convert.ToDouble(DGVExcel.Rows[s].Cells[5].Value));
                            cmd.Parameters.AddWithValue("@GTI", Convert.ToDouble(DGVExcel.Rows[s].Cells[6].Value));
                            cmd.Parameters.AddWithValue("@LessTE", Convert.ToDouble(DGVExcel.Rows[s].Cells[7].Value));
                            cmd.Parameters.AddWithValue("@LessPPH",
                                Convert.ToDouble(DGVExcel.Rows[s].Cells[8].Value));
                            cmd.Parameters.AddWithValue("@LessNTI",
                                Convert.ToDouble(DGVExcel.Rows[s].Cells[9].Value));
                            cmd.Parameters.AddWithValue("@TD", Convert.ToDouble(DGVExcel.Rows[s].Cells[10].Value));
                            cmd.Parameters.AddWithValue("@TWCE", Convert.ToDouble(DGVExcel.Rows[s].Cells[11].Value));
                            cmd.Parameters.AddWithValue("@TWPE", Convert.ToDouble(DGVExcel.Rows[s].Cells[12].Value));
                            cmd.Parameters.AddWithValue("@TATW", Convert.ToDouble(DGVExcel.Rows[s].Cells[13].Value));
                            cmd.Parameters.AddWithValue("@PersonID", (DGVExcel.Rows[s].Cells[14].Value));
                            cmd.ExecuteNonQuery();
                            Console.WriteLine(s);
                        }
                    }

                }
            }
            catch (OleDbException ex)
            {
                MessageBox.Show("Import error: " + ex);
            }
            finally
            {
                Restore();
            }

        }
    }

Answer 1

Con

Use full names. If it's a connection string then name it ConnectionString especially if it's out of scope and one cannot easily determine what it is.

---

    using (OleDbConnection conn = new OleDbConnection(Con))
    {
        using (OleDbCommand cmd = new OleDbCommand())
        {

You can join both usings together:

    using (OleDbConnection conn = new OleDbConnection(Con))
    using (OleDbCommand cmd = new OleDbCommand())
    {

---

for (int s = 0; s < DGVExcel.Rows.Count - 1; s++)

I don't see any reason for not using the i for the index. If it was the r it would at least stand for row but what does the s mean?

---

Insert INTO ACTB (FirstName, LastName,... VALUES(@First, @Last,

Why do you invent new names for the parameters? Isn't it easier to just copy/paste the one you already have for the columns?

Besides with so many colums consider using an array and build the query dynamically. You can create an array with a collection initializer like this:

var columns = new []
{
    "FirstName",
    "LastName",
    // ...
}

Then you join all names into a string separated by , as column names and another string also separated by commas but with the @ prefix for the values:

var sql =
    $"Insert INTO ACTB ({string.Join(", ", columns)} " +
    $"VALUES({string.Join(", ", columns.Select(x => $"@{x}"))}";

---

If you create the columns array with the names in the right order (like in your excel sheet) you can even reuse them to populate the parameters. Here's the complete example.

using (var conn = new OleDbConnection(Con))
using (var cmd = new OleDbCommand())
{
    cmd.Connection = conn;
    conn.Open();

    var columns = new[]
    {
        "FirstName",
        "LastName",
        // ...
    }

    var sql =
        $"Insert INTO ACTB ({string.Join(", ", columns)} " +
        $"VALUES({string.Join(", ", columns.Select(x => $"@{x}"))}";

    for (var i = 0; i < DGVExcel.Rows.Count - 1; i++)
    {
        cmd.Parameters.Clear();
        for (var j = 0; j < columns.Length; j++)
        {
            cmd.Parameters.AddWithValue($"@{columns[j]}", DGVExcel.Rows[i].Cells[j].Value);
        }

        cmd.ExecuteNonQuery();
        Console.WriteLine(i);
    }
}

Answer 2

Here are some minor suggestions in addition to t3chb0t's:

Using var

OleDbCommand cmd = new OleDbCommand() could easily be var cmd = new OleDbCommand() and not lose any readability at all.

Nesting

All that nesting can sometimes make it hard to follow scope. In some cases you might choose to flip your if-statements and return early. This doesn't work perfectly for all cases but I think it would work just fine here. if (dr == DialogResult.OK) { ... } would become if (dr != DialogResult.OK) { return; } and now your entire try-catch is no longer nested further to the right.