16

I have got this code which reads data from SQL DB.

I don't know how should I edit it so I can use original column name and not column index.

string query = "SELECT * FROM zajezd WHERE event='" + thisrow+ "' AND year='" + klientClass.Year() + "'";
SqlCommand cmd= new SqlCommand(query, spojeni);
spojeni.Open();
SqlDataReader read= cmd.ExecuteReader();
            

if (read.Read())
{
    maskedTextBox2.Text = read.IsDBNull(24) ? 
        string.Empty : 
        read.GetDateTime(24).ToString("MM/dd/yyyy");
4
  • 3
    So you're using SELECT * and the column you're after is the 24th column? This is a terrible way to code - what happens when someone changes the table (e.g. adds a column somewhere between 1-23)? You might get an error, or you might happen to get a different date time column and not notice. Also, do you like SQL injection? Please use parameterized queries. Dynamic SQL like this is why there are so many SQL injection exploits on the web every day. Commented Sep 27, 2013 at 19:45
  • @NathanKoop in your edit you have added a closing brace. Probably you are right, but as you can see from the comments below this give a very specific meaning to the query used by the OP. If the OP needs only one column then a better approach could be possible. Commented Sep 27, 2013 at 19:57
  • @Steve sorry for the delayed response. I don't see the comment(s) that you are referencing, but I have removed the curly. It's not a big deal to me either way if it's there or not :-) Commented Sep 30, 2013 at 13:38
  • 1
    @NathanKoop we were debating if a simple ExecuteScalar would be enough for the OP or not. ExecuteScalar is the preferred way to go when you have to retrieve only one column from one row. The missing brace has been taken (by me) as a sign that the OP have other code following the single field read, and in that case the ExecuteScalar is not the right tool. With the closing brace, my answer still stands, but ExecuteScalar makes more sense as Scott Chamberlain pointed in its comment in my answer. Not a big deal to me either. Commented Sep 30, 2013 at 13:48

4 Answers 4

26

You are looking for SqlDataReader.GetOrdinal

According to MSDN

Gets the column ordinal, given the name of the column.

if (read.Read())
{
   int colIndex = read.GetOrdinal("MyColumnName");
   maskedTextBox2.Text = read.IsDBNull(colIndex) ? 
                  string.Empty : 
                  read.GetDateTime(colIndex).ToString("MM/dd/yyyy");

}

As a side note, your query is open to sql injection. Do not use a string concatenation to build a sql command but use a parameterized query

  string query = "SELECT * FROM zajezd WHERE event=@p1 AND year=@p2";
  using(SqlCommand cmd= new SqlCommand(query, spojeni))
  {
     spojeni.Open();
     cmd.Parameters.AddWithValue("@p1",thisrow);
     cmd.Parameters.AddWithValue("@p2",klientClass.Year().ToString());
     using(SqlDataReader read= cmd.ExecuteReader())
     {
       ......
     }
  }
Sign up to request clarification or add additional context in comments.

5 Comments

I did not downvote, but if I was doing it I would do the read outside of the loop. EDIT: Just saw this is a if statement not a loop. I guess he should use ExecuteScaler instead.
We are assuming that the OP needs only one column, but this cannot be verified from the incomplete code posted, however if the OP really need only one column from one record I concur then that the whole query should be rewritten
Very true, did not think of that.
not my d/v, but I did get a downvote on a similar style question where I did not explicitly state that the OP should not use this style of SQL. That may have been the reason, and I see that you've added it now.
For performance reasons, Microsoft now recommends you call your GetOrdinal outside your read loop, and only call GetDateTime(int) inside the loop
9

I would try (string)(reader["ColumnName"] == DBNull.Value ? "" : reader["ColumnName"]); to do it by the column name.

3 Comments

This is what we do and works fine (string)(reader["ColumnName"] == DBNull.Value ? "" : reader["ColumnName"]);
Well, that's correct, but why don't you add to your answer then?
I did not know thats what he was wanting not a big deal though i will edit it and yours worked for him.
3

You could use GetOrdinal to create your own IsDBNull(string name) extension method.

[DebuggerStepThrough]
public static class SqlDataReaderExtensions
{
    /// <summary>Gets a value that indicates whether the column contains non-existent or missing values.</summary>
    /// <param name="name">The name of the column. </param>
    /// <returns> <see langword="true" /> if the specified column value is equivalent to <see cref="T:System.DBNull" />; otherwise <see langword="false" />.</returns>
    /// <exception cref="T:System.IndexOutOfRangeException">The name specified is not a valid column name. </exception>
    public static bool IsDBNull(this SqlDataReader reader, string name)
    {
        int columnOrdinal = reader.GetOrdinal(name);
        return reader.IsDBNull(columnOrdinal);
    }
}

// Usage
if (read.Read())
{
    maskedTextBox2.Text = read.IsDBNull("MyColumnName") ? 
                  string.Empty : 
                  read.GetDateTime("MyColumnName").ToString("MM/dd/yyyy");

}

Comments

-1

You can use GetOrdinal with isnull property like this:

SqlDataReader reader = await cmd.ExecuteReaderAsync();
model.fieldName = reader.IsDBNull(reader.GetOrdinal("fieldName")) ? false : Convert.ToBoolean(reader["fieldName"]);

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.