C#(loading search results in textbox)

Question

I am trying to search the database and set the results in textboxes. I am getting error, which says "invalid cast exception". I need your guide please.

private void btn_search_Click(object sender, EventArgs e)
{
    con.Open();
    string STR="select * from TICKETSALES where REFERENCE="+txtSearch.Text;
    cmd = new SqlCommand(STR,con );
    dr = cmd.ExecuteReader();
    if(dr.Read())
    {
        txtTrans.Text = dr.GetInt32("TRANSACTIONNUMBER").ToString();
        txtPax.Text = dr.GetString("PASSENGERNAME");
    }
    else
    {
        MessageBox.Show("Ticket Number not Found");
    }
}

Well, before looking at the main problem, there is a real one, don't use the query like that, I mean don't pass the value directly in the query string because you are totally open to SQL Injection. use parameters to stay in the safe side ! — Kaj, Jun 13 '18 at 10:01
if the field `REFERENCE` is of type `string` (i.e. `varchar/nvarchar`) then you should use `REFERENCE='"+txtSearch.Text+"'`. As every string should be kept inside a single quote('). — vikscool, Jun 13 '18 at 10:03
I can see so many problems with this code it hurts my eyes. Don't use underscore in identifier names (like `btn_search` I assume you have). Don't put SQL in your presentation layer (and do have layers to separate presentation from logic from data access). Don't use `SELECT *`, use specific column names instead. Don't concatenate user input with SQL. Don't use all-caps for identifier names, use camelCase. Add spaces after commas, and not before parenthesis (or add spaces after the opening parenthesis too). hth. — Tsahi Asher, Jun 13 '18 at 10:12
@TsahiAsher, your comments shows instructions for clean code. Kudos to your suggestions. I will follow your suggestions for my projects. Thanks — Prasad Telkikar, Jun 13 '18 at 10:24
Possible duplicate of [What are good ways to prevent SQL injection?](https://stackoverflow.com/questions/14376473/what-are-good-ways-to-prevent-sql-injection) — mjwills, Jun 13 '18 at 11:16

Prasad Telkikar · Accepted Answer · 2018-06-13T10:41:10.260

1

Modify your select statement to get required column details.
While assigning values to test box, use column index to get value from dr
Convert value to string assign it to respective textbox

Here is sample implementation

con.Open();
//Use of camelCasing. transactionNumber instead of TRANSACTIONNUMBER
string STR="select transactionNumber,passengerNumber from TICKETSALES where REFERENCE=@search";
cmd = new SqlCommand(STR,con );
cmd.Parameters.Add("@search", txtSearch.Text);
dr = cmd.ExecuteReader();
if(dr.Read())
    {
        txtTrans.Text = Convert.ToString(dr[0]);
        txtPax.Text = Convert.ToString(dr[1]);
    }

Few tips for best coding practices (Credits: @tsahi-asher)

Don't pass values inside query, use parameters in query and use Paramere.Add() function to replace parameter with its value
Don't put your sql statements in presentation layer. Have some dedicated layer of SQL.
Don't use select *, use specific column name.
Don't use all-caps for identifier names, use camelCase.

edited Jun 13 '18 at 10:41

answered Jun 13 '18 at 10:01

Prasad Telkikar

15,207
5
21
44

2

We still talking about SQL injection, and you made the same mistake and encourage him to keep going in the wrong way ! – Kaj Jun 13 '18 at 10:04
Please provide reason for downvote so I can improve my answer – Prasad Telkikar Jun 13 '18 at 10:09
I'm not your downvoter but I believe it's all about the old answer and prevent sql injection – Kaj Jun 13 '18 at 10:13
It was me. I upvoted again. I actually wanted to comment on why, but SO blocked my comment, saying I shouldn't comment on my downvote, but only on answer improvement! – Tsahi Asher Jun 13 '18 at 10:14

B.Hawkins · Answer 2 · 2018-06-13T10:20:54.367

How about something like this:

Note the sql injection protection by paramtising the sql query.

private void btn_search_Click(object sender, EventArgs e)
{
    using (SqlConnection con = new SqlConnection(connectionString))
    {
        string query = "select top 1 TRANSACTIONNUMBER, PASSENGERNAME from ticketsales where reference=@ref";
        using (SqlDataAdapter adap = new SqlDataAdapter(query, con))
        {
            con.Open();
            DataTable dt = new DataTable();
            adap.SelectCommand.Parameters.AddWithValue("@ref", txtSearch.Text.Trim());
            adap.Fill(dt);

            if (dt.Rows.Count > 0)
            {
                txtTrans.Text = dt.Rows[0]["TRANSACTIONNUMBER"].ToString().Trim();
                txtPax.Text = dt.Rows[0]["PASSENGERNAME"].ToString().Trim();
            }
            else
            {
                MessageBox.Show("Ticket Number not Found");
            }
        }
    }
}

Michał Turczyn · Answer 3 · 2018-06-13T10:32:02.697

There are few issue already mentioned in comments and posts. I will chip in my remarks - you don't dispose of unmanaged resources, one answer covers that, but it violates your code. So here is alternative solution:

SqlConnection con;
SqlCommand cmd;
SqlDataReader dr;

//some methods, fields

private void btn_search_Click(object sender, EventArgs e)
{
    con.Open();
    // as it has benn already said, you have to prevent yourself from SQL injection!
    cmd = (new SqlCommand("select * from TICKETSALES where REFERENCE=@ref", con)).Parameters.AddWithValue("@res", txtSearch.Text.Trim());
    dr = cmd.ExecuteReader();

    if (dr.Read())
    {
        txtTrans.Text = dr.GetInt32("TRANSACTIONNUMBER").ToString();
        txtPax.Text = dr.GetString("PASSENGERNAME");
    }
    else
    {
        MessageBox.Show("Ticket Number not Found");
    }
}
// it looks like you have unamanaged resources held by fields in your form,
// so to release them you have to call their Dispose() method!
// normally you should use using keyword if they were used locally in a method, as other answer states
public void Dispose()
{
    base.Dispose();
    if (con != null) con.Dispose();
    if (cmd != null) cmd.Dispose();
    if (dr != null) dr.Dispose();
}

C#(loading search results in textbox)

3 Answers3