How can i make my database method more future proof?

Viewed 58

I am currently working on a new application with a database data transfer between two databases. I currently have a method written, but it looks like I can make this code way more compact. I read an article that used the Using Statement, but I'm not sure how I can implement this in my code. Furthermore, I would like to address that I have the following public declarations:

private IDbConnection ExportConnection = new OleDbConnection(Helper.CnnVal("ExportDatabase"));
    private IDbConnection SchoolConnection;
    private DataTable exportDb = new DataTable();
    private DataTable schoolDb = new DataTable();
    private OleDbCommandBuilder cb;
    private OleDbDataAdapter dataAdapterSchool;
    private OleDbDataAdapter dataAdapterExport;
    private OleDbCommand dbCommand;
    private string ZoekQuery { get; set; }
    private DataAccess db = new DataAccess();
    private string SchoolConnectionString;
    private string selectSchoolQuery;

This is the method:

public void InlezenClientGegevens()
{
    if ((bool)Checbox.IsChecked)
    {
        SchoolConnectionString = $@"I removed the string for privacy reasons";
        try
        {
            ExportConnection.Open();
            ZoekQuery = $"SELECT * FROM Client WHERE Cliëntnummer BETWEEN '{TxtVan.Text}' AND '{TxtTm.Text}'";
            dbCommand = new OleDbCommand(ZoekQuery, (OleDbConnection)ExportConnection);
            dataAdapterExport = new OleDbDataAdapter(ZoekQuery, (OleDbConnection)ExportConnection);
            dbCommand.ExecuteNonQuery();
            dataAdapterExport.AcceptChangesDuringFill = false;
            dataAdapterExport.Fill(exportDb);
            ExportConnection.Close();
        }
        catch (Exception ex)
        {
            MessageBox.Show(ex.Message);
        }
        SchoolConnection = new OleDbConnection(SchoolConnectionString);
        try
        {
            SchoolConnection.Open();
            selectSchoolQuery = "SELECT * FROM Info";
            dbCommand = new OleDbCommand(selectSchoolQuery, (OleDbConnection)SchoolConnection);
            dataAdapterSchool = new OleDbDataAdapter(selectSchoolQuery, (OleDbConnection)SchoolConnection);
            dbCommand.ExecuteNonQuery();
            dataAdapterSchool.FillSchema(schoolDb, SchemaType.Source);
            dataAdapterSchool.Fill(schoolDb);
        }
        catch (Exception ex)
        {
            MessageBox.Show(ex.Message);
        }
        try
        {
            cb = new OleDbCommandBuilder(dataAdapterSchool);
            cb.GetUpdateCommand();
            schoolDb.Merge(exportDb, false, MissingSchemaAction.Add);
            dataAdapterSchool.Update(schoolDb);
        }
        catch (Exception ex)
        {
            MessageBox.Show(ex.Message);
        }


    }
}

EDIT:

public void InlezenClientGegevens()
{
    if ((bool)Checkbox.IsChecked)
    {
        try
        {
            using (var connection = new OleDbConnection(Helper.CnnVal("ExportDatabase")))
            {
                var ZoekQuery = $"SELECT * FROM client WHERE Cliëntnummer BETWEEN '{TxtVan.Text}' AND '{TxtTm.Text}'";
                using (var dbcommand = new OleDbCommand(ZoekQuery, connection))
                {
                    using (var exportAdapter = new OleDbDataAdapter(ZoekQuery, connection))
                    {
                        exportAdapter.AcceptChangesDuringFill = false;
                        exportAdapter.Fill(exportDb);
                    }
                }
            }
            using (var connection = new OleDbConnection($"))
            {
                var ZoekQuery = "SELECT * FROM info";
                using (var dbcommand = new OleDbCommand(ZoekQuery, connection))
                {

                    using (var SchoolAdapter = new OleDbDataAdapter(ZoekQuery, connection))
                    {
                        SchoolAdapter.FillSchema(schoolDb, SchemaType.Source);
                        SchoolAdapter.Fill(schoolDb);
                        using (var cb = new OleDbCommandBuilder(SchoolAdapter))
                        {
                            cb.GetUpdateCommand();
                            schoolDb.Merge(exportDb, false, MissingSchemaAction.Add);
                            SchoolAdapter.Update(schoolDb);
                        }
                    }
                }
            }
        }
        catch (Exception ex)
        {
            MessageBox.Show(ex.Message);
        }          
    }
}
1 Answers

You dont need to open close the connection if you use DataAdappter.Fill. I would also use the using statement for everything implementing IDisposable(like for the OleDbConnection and OleDbCommand, not necessary for the DataTable). So you should create the connection in the method.

You should also use parameters for your queries instead of string concatenation to avoid sql-injection. You should also try-parse the input first, to see if it's really an int:

public void InlezenClientGegevens()
{
    bool validVan = int.TryParse(TxtVan.Text, out int van);
    bool validTm = int.TryParse(TxtTm.Text, out int tm);
    if (!validVan || !validTm)
    {
        // inform user
        return;
    }

    if (Checkbox.IsChecked)
    {
        try
        {
            using (OleDbConnection connection = new OleDbConnection(Helper.CnnVal("ExportDatabase")))
            {
                string zoekQuery = $"SELECT * FROM client WHERE Cliëntnummer BETWEEN ? AND ?";
                using (OleDbCommand dbCommand = new OleDbCommand(zoekQuery, connection))
                {
                    dbCommand.Parameters.Add(new OleDbParameter("Van", OleDbType.Integer).Value = van);
                    dbCommand.Parameters.Add(new OleDbParameter("Tm", OleDbType.Integer).Value = tm);
                    using (OleDbDataAdapter exportAdapter = new OleDbDataAdapter(dbCommand))
                    {
                        exportAdapter.Fill(exportDb);
                    }
                }
            }
            // ...
        }
        catch (Exception ex)
        {
            MessageBox.Show(ex.Message);
        }
    }
}
Related