patterncsharpMinor
Query and data manipulation
Viewed 0 times
andquerydatamanipulation
Problem
The below sections of code are part of a list administration project I am working on to teach myself C#. This is from a tab that is used for list administration. There is a drop down box that allows selection of list administrators, which populates a dropdown box with available lists.
Once a list is selected, it populates two listboxes with users associated with the list administrators organization (such as a dance school), one has users that are in the list, the others are ones not in the list.
Before I start coding the moving of users between boxes and updating the tables, have I approached the methodology in the most efficient way? I know it works, but is there a method of setting it up I should look at to make things easier as I move forward?
Code for list selection and listbox population:
```
/*
* DATA MANIPULATION AND FORM ROUTINES FOR THE LISTS TAB
*
*/
private void btnEditList_Click(object sender, EventArgs e)
{
string disp = cmbListAdmins.SelectedValue.ToString();
string listQuery = "SELECT * FROM test.tbl_lists WHERE test.tbl_lists.user_id = " + disp + ";";
dbconn = new MySqlConnection(connString);
MySqlDataAdapter listadapter = new MySqlDataAdapter(listQuery, dbconn);
DataTable dtAvailLists = new DataTable();
try
{
listadapter.Fill(dtAvailLists);
dtAvailLists.Columns.Add("FullName", typeof(string), "list_name + ' ' + list_description");
BindingSource bindsource1 = new BindingSource();
bindsource1.DataSource = dtAvailLists;
cmbListSelect.DataSource = bindsource1;
cmbListSelect.DisplayMember = "FullName";
cmbListSelect.ValueMember = "list_id";
cmbListSelect.Enabled = true;
}
catch (Exception err)
{
File.AppendAllText(logFileName,
string.Format("{0}: Unable to fill list admin combo: Query is {1}, result is {2}.",
DateTime.Now,
listQuery,
err,
System.Environment.NewLine));
}
Once a list is selected, it populates two listboxes with users associated with the list administrators organization (such as a dance school), one has users that are in the list, the others are ones not in the list.
Before I start coding the moving of users between boxes and updating the tables, have I approached the methodology in the most efficient way? I know it works, but is there a method of setting it up I should look at to make things easier as I move forward?
Code for list selection and listbox population:
```
/*
* DATA MANIPULATION AND FORM ROUTINES FOR THE LISTS TAB
*
*/
private void btnEditList_Click(object sender, EventArgs e)
{
string disp = cmbListAdmins.SelectedValue.ToString();
string listQuery = "SELECT * FROM test.tbl_lists WHERE test.tbl_lists.user_id = " + disp + ";";
dbconn = new MySqlConnection(connString);
MySqlDataAdapter listadapter = new MySqlDataAdapter(listQuery, dbconn);
DataTable dtAvailLists = new DataTable();
try
{
listadapter.Fill(dtAvailLists);
dtAvailLists.Columns.Add("FullName", typeof(string), "list_name + ' ' + list_description");
BindingSource bindsource1 = new BindingSource();
bindsource1.DataSource = dtAvailLists;
cmbListSelect.DataSource = bindsource1;
cmbListSelect.DisplayMember = "FullName";
cmbListSelect.ValueMember = "list_id";
cmbListSelect.Enabled = true;
}
catch (Exception err)
{
File.AppendAllText(logFileName,
string.Format("{0}: Unable to fill list admin combo: Query is {1}, result is {2}.",
DateTime.Now,
listQuery,
err,
System.Environment.NewLine));
}
Solution
It seems to me that if
The
File.AppendAllText throws an exception dbConn won't be closed.The
dbConn.Close() should be in a finally block. (Exception inside catch block.)Context
StackExchange Code Review Q#13204, answer score: 6
Revisions (0)
No revisions yet.