Shortening data access code... would this work?

Question

I am querying for data, if the data does not exist, I insert it. if it does, I do something else:

SqlCommand checkHead = new SqlCommand("SELECT * FROM TABLE WHERE ORDER_NO = '" + orderNo + "';", connection);
SqlDataReader checkHeadReader = checkHead.ExecuteReader(CommandBehavior.SingleRow);

if (!checkHeadReader.HasRows)
{
    checkHeadReader.Close();
    addHead.ExecuteNonQuery();
}

But I wonder if there's a shorter way to code this? would the code below work?

SqlCommand checkHead = new SqlCommand("SELECT * FROM TABLE WHERE ORDER_NO = ' + orderNo + "';", connection);

if(checkHead.ExecuteReader(CommandBehavior.SingleRow).HasRows)
    addHead.ExecuteNonQuery();
else //this order already exists
    Server.Transfer(@"~/Views/Error.aspx");

Close the reader, check for sql injection, don't use select * but select count(*) or exists. — Carra, Dec 11 '12 at 15:40
It looks like you are executing the `addHead` command if there are NO rows in the first example and executing it only if there ARE rows in the second example. Other than that they look more or less equivalent. Assuming you have a compiler and a computer to use, the best way to find out if it works is to try it yourself. — pseudocoder, Dec 11 '12 at 15:41
@SLaks won't the reader close automatically when it falls out of scope? If it's a big deal can you link to an explanation? — pseudocoder, Dec 11 '12 at 15:42
sorry i guess my question came across as lazy... i really after weather or not the code is syntactically correct? i wasn't sure if i could use the executeReader in the if statement, esp. without instantiating a SqlDataReader object first? Also i didn't think i'd need to paramatarize the statement because the variable you see order order number is generated in code and never comes from the webform. I'll try count(*)... is this quicker? — Stuart, Dec 11 '12 at 15:44
@pseudocoder: No; it will be GC'd some time later. http://msdn.microsoft.com/en-us/library/ms182289.aspx — SLaks, Dec 11 '12 at 15:44
@Stuart: Your first code does not instantiate a DataReader; it merely creates a variable to hold a DataReader. `ExecuteReader()` instantiates the DataReader. — SLaks, Dec 11 '12 at 15:46

Ciarán · Accepted Answer · 2012-12-11T15:45:19.083

3

ExecuteScalar is great for this, E.g.

using (SqlCommand cmdCheck = new SqlCommand("Select Count(*) From Table Where Order_No = '" + orderNo + "'", connection))
{
    int nExists = (int)cmdCheck.ExecuteScalar();
    if (nExists==0) addHead.ExecuteNonQuery();
}

edited Dec 11 '12 at 15:45

answered Dec 11 '12 at 15:37

Ciarán

3,017
1
16
20

1

This is in fact a shorter way to do this and also quite neat, though I suppose you could argue there's a line missing that does the addHead. – Ciarán Dec 11 '12 at 15:43
But the first line won't compile – RvdK Dec 11 '12 at 15:45
You are quite right, there's a typo which I have amended, but it does answer the question and you should not have been so quick to -1 me. – Ciarán Dec 11 '12 at 15:46
this looks like a much neater solution... at this stage in the code i'm already in a using{} statement, can you nest using statements? – Stuart Dec 11 '12 at 15:54
It's not stupid :-) we all have to learn and it's a pleasure. – Ciarán Dec 11 '12 at 16:05
@lynamc: indeed I saw only the copied typo. Removed my -1 because executeScalar looks good indeed :) – RvdK Dec 11 '12 at 16:38

Shortening data access code... would this work?

1 Answers1