Reputation: 22321
I forget to return value in single tier application.
public int Studentid()
{
try
{
SqlConnection con = new SqlConnection(connectionStr);
SqlCommand cmd = new SqlCommand("SELECT s_id FROM student where name = + ('" + Request.QueryString.ToString() + "')", con);
con.Open();
SqlDataReader dr = null;
con.Open();
dr = cmd.ExecuteReader();
if (dr.Read())
{
//Want help hear how I return value
}
con.Close();
}
catch (Exception ex)
{
throw ex;
}
}
Upvotes: 7
Views: 31459
Reputation: 6812
Like this?
public int Studentid()
{
int studentId = -1;
SqlConnection con = null;
try
{
con = new SqlConnection(connectionStr);
SqlCommand cmd = new SqlCommand("SELECT s_id FROM student where name = + ('" + Request.QueryString.ToString() + "')", con);
SqlDataReader dr = null;
con.Open();
dr = cmd.ExecuteReader();
if (dr.Read())
{
studentId = dr.GetInt32(0);
}
dr.Close();
}
catch (Exception ex)
{
throw ex;
}
finally
{
if(con != null)
con.Close();
con = null;
}
return studentId;
}
Upvotes: -1
Reputation: 18652
The easiest way to return a single value is to call ExecuteScalar
. You should also fix your SQL injection bug. And did you mean to encode the entire query string array, or just to pick out a single value?
public int StudentId()
{
string sql = "SELECT s_id FROM student WHERE name = @name";
using (var con = new SqlConnection(connectionStr))
{
using (var cmd = new SqlCommand(sql, con))
{
cmd.Parameters.Add("@name", DbType.VarChar, 256).Value = Request.QueryString["name"];
con.Open();
return (int)cmd.ExecuteScalar();
}
}
}
Upvotes: 3
Reputation: 700910
You should use using
blocks, so that you are sure that the connection, command and reader are closed correctly. Then you can just return the value from inside the if
statement, and doesn't have to store it in a variable until you have closed the objects.
You only have to open the connection once.
You should use parameterised queries, instead of concatenating values into the query.
public int Studentid() {
try {
using (SqlConnection con = new SqlConnection(connectionStr)) {
using (SqlCommand cmd = new SqlCommand("SELECT s_id FROM student where name = @Name", con)) {
cmd.Parameters.Add("@Name", DbType.VarChar, 50).Value = Request.QueryString.ToString();
con.Open();
using (SqlDataReader dr = cmd.ExecuteReader()) {
if (dr.Read()) {
return dr.GetInt32(0);
} else {
return -1; // some value to indicate a missing record
// or throw an exception
}
}
}
}
} catch (Exception ex) {
throw; // just as this, to rethrow with the stack trace intact
}
}
Upvotes: 4
Reputation: 311375
Here is a version of your method that achieves what you're after.
public int GetStudentId()
{
var sql = string.Format("SELECT s_id FROM student where name = '{0}'", Request.QueryString);
using (var con = new SqlConnection(connectionStr))
using (var cmd = new SqlCommand(sql, con))
{
con.Open();
var dr = cmd.ExecuteReader();
return dr.Read() ? return dr.GetInt32(0) : -1;
}
}
There's no need to use try/catch when you don't do anything with the exception except re-throw (and in fact you were losing the original stack trace by using throw ex;
instead of just throw;
. Also, the C# using
statement takes care of cleaning up your resources for you in fewer lines of code.
IMPORTANT
Passing the query string directly into SQL like that means that anyone can execute random SQL into your database, potentially deleting everything (or worse). Read up on SQL Injection.
Upvotes: 21
Reputation: 218952
int studId=0;
if(rdr.Read())
{
studId=rdr.GetInt32(rdr.GetOrdinal("s_id"));
}
Upvotes: 1
Reputation: 30127
if (dr.Read())
{
//Want help hear how i return value
int value = dr.GetInt32("s_id");
}
Upvotes: 0