I need to insert values into several tables first I have to retrieve university id from table college and then insert faculty name into table faculty and get generated by SQL Server ID. After all of this I have to insert both ids into an other table.
Problem is that I have to close readers and after I do it I can’t retrieve those ids from them so variable where they should be saved is null. Here is my code. How to do it correctly?
Sorry I am new to C# and SQL Server.
// reading data into combobox
try
{
SqlDataReader myReader = null;
SqlCommand myCommand = new SqlCommand("select * from colege", myConnection);
myReader = myCommand.ExecuteReader();
while (myReader.Read())
{
comboBox1.Items.Add(myReader["name"].ToString());
// Console.WriteLine(myReader["Column2"].ToString());
}
}
catch (Exception ex)
{
Console.WriteLine(ex.ToString());
}
myConnection.Close();
private void button1_Click(object sender, EventArgs e)
{
string item = comboBox1.Text.ToString();
// MessageBox.Show(item);
SqlConnection myConnection = new SqlConnection("user id=bogdan_db;" +
"password=1234;server=localhost;" +
"Trusted_Connection=yes;" +
"database=cafedrascience; " +
"connection timeout=30");
try
{
myConnection.Open();
}
catch (Exception E)
{
Console.WriteLine(E.ToString());
}
// reading data into combobox
String colegeid = null;
try
{
SqlDataReader myReader = null;
SqlCommand myCommand = new SqlCommand("select * from colege where name like'" + item + "'", myConnection);
myReader = myCommand.ExecuteReader();
while (myReader.Read())
{
colegeid = myReader["id"].ToString();
// Console.WriteLine(myReader["Column2"].ToString());
}
myReader.Close();
}
catch (Exception ex)
{
MessageBox.Show(ex.ToString());
}
String facultyid = null;
try
{
SqlDataReader myReader1 = null;
SqlCommand myCommand = new SqlCommand("select * from depart where name like'" + textBox1.Text + "'",
myConnection);
myReader1 = myCommand.ExecuteReader();
while (myReader1.Read())
{
facultyid = myReader1["id"].ToString();
}
myReader1.Close();
}
catch (Exception ex)
{
MessageBox.Show(ex.ToString());
}
SqlCommand myCommand1 = new SqlCommand("INSERT INTO coledge_faculty (coledge_id, faculty_id) " +
"Values ('"+colegeid+"''"+facultyid+"')", myConnection);
myCommand1.ExecuteNonQuery();
// MessageBox.Show(colegeid);
// MessageBox.Show(facultyid);
myConnection.Close();
}
Advertisement
Answer
The number one thing I can stress about your code is that you should be using parameterised queries, beyond the obvious risks of SQL Injection, it also protects you against malformed SQL, data truncation through conversion, and it allows you to use cached execution plans.
The next thing to point out is that you should not be using SELECT * in production code, e.g.
SqlCommand myCommand = new SqlCommand("select * from colege where name like'" + item + "'", myConnection);
myReader = myCommand.ExecuteReader();
while (myReader.Read())
{
colegeid = myReader["id"].ToString();
// Console.WriteLine(myReader["Column2"].ToString());
}
Why bother retrieving all the columns of colege from the database, then sending them all over the network if you only care about the column id?
Finally your diagnosis of the problem is not correct:
Problem is that I have to close readers and after I do it, I can’t retrieve those ids from them, so variable where they should be saved is null
If you assign the string variable colegeid a value you have retrieved from a data reader, it will not be null after you have closed the reader, it will retain the value you assigned. The most likely reason the variable is null is because your reader returns no rows so you never assign it a value.
Now, rant over, I will actually answer your question. You are massively over complicating the issue, you do not need to retrieve the values into your application tier only to insert them to another table, you can do this all in a single query in your database:
INSERT INTO coledge_faculty (coledge_id, faculty_id)
SELECT c.id, d.id
FROM depart AS d
CROSS JOIN colege AS c
WHERE d.Name = @Depart
AND c.Name = @Colege;
Then it would just be a case of calling this SQL from C#:
string item = comboBox1.Text.ToString();
string connectionString = "user id=bogdan_db; password=1234;server=localhost; Trusted_Connection=yes; database=cafedrascience; connection timeout=30";
string sql = @"INSERT INTO coledge_faculty (coledge_id, faculty_id)
SELECT c.id, d.id
FROM depart AS d
CROSS JOIN colege AS c
WHERE d.Name = @Depart
AND c.Name = @Colege;";
using (var connection = new SqlConnection(connectionString))
using (var command = new SqlCommand(sql, connection))
{
command.Parameters.Add("@Colege", SqlDbType.VarChar, 50).Value = item;
command.Parameters.Add("@Depart", SqlDbType.VarChar, 50).Value = textBox1.Text;
connection.Open();
command.ExecuteNonQuery();
}
It is usually a good idea to use using blocks with objects that implement IDisposable, this will ensure the resources are freed up when you are done with them (Don’t confuse this with not being able to reuse the connection, .NET has connection pooling in the background so it will reuse connections for you, you shouldn’t keep your SqlConnection object open available in case you need to use it again).
On another unrelated note, I also think you are too liberal with try/catch blocks, or at least not dealing with the exception properly, using this one as an example:
try
{
myConnection.Open();
}
catch (Exception E)
{
Console.WriteLine(E.ToString());
}
If myConnection.Open() does throw an error, you still carry on with the method. You will carry on until you get to here:
SqlCommand myCommand = new SqlCommand("select * from colege where name like'" + item + "'", myConnection);
myReader = myCommand.ExecuteReader();
Where you will get another exception, something along the lines of the command requiring an open and available SqlConnection, so you go to the exception.
catch (Exception ex)
{
MessageBox.Show(ex.ToString());
}
Again you don’t exit the method, but carry on, and you will get the same error later when you try and use the connection again. Again the method carries on and you will use the closed connection a third time to try and insert two variables that were never assigned because exceptions were thrown into your database. Fine, use try catch blocks, but do something meaningful with the exception and exit the method.