SqlDataReader.Read() always false in C#

Question

I have a Table in my local database Ships(HistID,ShipName,ShipLength). I am polling the database for all ships with HistID == theme, but while(reader.Read()){} is never entered. Also, my Ships table has more than one row (saw this problem in another SO question) so I'm not sure why I cant store the results into List of Tuples. Executing the query alone in Visual Studio 2015 yields the correct results.

     public List<Tuple<String, int>> getHistoricalShipList(int theme)
        {
            List<Tuple<String, int>> list = new List<Tuple<string, int>>();

            using (db)
            {
                   cmd = new SqlCommand(@"Select ShipName, ShipLength from Ships Where HistID=@theme", db);
                   cmd.Parameters.AddWithValue("@theme", theme);

                db.Open();

                SqlDataReader reader = cmd.ExecuteReader();

                if (reader.HasRows) // always returning false
                {
                    //Loop through results
                    while (reader.Read())
                    {
                        String shipName = reader[0].ToString();
                        int shipLength = Convert.ToInt32(reader[1]);
                        list.Add(Tuple.Create(shipName, shipLength));
                    }
                }
                db.Close();
            }

            return list;
        }

EDIT:: removed the single quotes from the query as suggested, but still having the same issue.

What is the type of HistID column? Sounds like numeral based on it's name. You should always use parameterized queries by the way. This kind of string concatenations are open for SQL Injection attacks. — Soner Gönül
– Soner Gönül, Commented Feb 25, 2016 at 15:50
There is so many things going on in your code, that I don't know where to start. Check your using blocks. You have not enough and at least the one for db looks fishy. You should use parameters instead of string format for your query. And there is no point in having an if(hasRows) when you have a while loop around your read. — nvoigt
– nvoigt, Commented Feb 25, 2016 at 15:52
Use parametrized query, not string.Format. Does the generated query return any results outside your program? — Vojtěch Dohnal
– Vojtěch Dohnal, Commented Feb 25, 2016 at 15:52
Yes Vojtech, When I open up a Query editor in VS, the query returns what I need it to return. I am beginner, please excuse my terrible coding! — Tysia Krepps
– Tysia Krepps, Commented Feb 25, 2016 at 15:57

Habib · Accepted Answer · 2016-02-25 15:54:07Z

3

Your theme is of type int, and you are enclosing it in single quotes like it is a string value. Remove the quotes, but more importantly, use Parameters

cmd = new SqlCommand(string.Format(@"Select ShipName, ShipLength from Ships Where HistID={0}", theme), db);

Never use string concatenation/string format to build SQL statements, your code is prone to SQL injection.

cmd = new SqlCommand(@"Select ShipName, ShipLength from Ships Where HistID=@theme", db);
cmd.Parameters.AddWithValue("@theme", theme);
//Or more precise 
//cmd.Parameters.Add(new SqlParameter("@theme", SqlDbType.Int) {Value = theme});

The reason you are not getting any rows back is, that your field HistID is of numeric type and you are trying to compare it with a string value (by enclosing the value in single quote).

edited Feb 25, 2016 at 15:54

answered Feb 25, 2016 at 15:50

Habib

224k30 gold badges420 silver badges446 bronze badges

Sign up to request clarification or add additional context in comments.

5 Comments

Soner Gönül Over a year ago

AddWithValue? Ouchh.. :)

Tysia Krepps Over a year ago

Oh no! Your right, I missed that. However, I updated that line and I'm still reader.Read() is still false. Updated OP with your correction.

Mark Brackett Over a year ago

Agreed w/everything except that this is the OP's actual problem; because of implicit conversion, the query would still work.

Scott Chamberlain Over a year ago

@SonerGönül There is nothing wrong with AddWithValue if you are using it on a non variable length or non ambiguous type. Sure, for strings and byte[] I would never use it, but for a int, there is nothing wrong with that.

Soner Gönül Over a year ago

@ScottChamberlain Yes, this isn't a problem in this case but as a general recommendation, specially explicitly specifying DbType, makes code much more readable and understandable in my opinion.

Mark Brackett · Accepted Answer · 2016-02-25 16:00:23Z

1

Remove HasRows check and just use the .Read while loop; there's various bug reports on HasRows not being entirely accurate in some cases.

Other than that, it's likely you're making a mistake somewhere. Either theme isn't what you expect it to be, or some environment error like hitting the wrong database.

answered Feb 25, 2016 at 16:00

Mark Brackett

85.9k17 gold badges113 silver badges156 bronze badges

1 Comment

Tysia Krepps Over a year ago

I hate it when I blunder this bad. Theme was being passed in as 0 always, which doesn't exist in my DB. I hard-coded a 1 in (which exists) and the problem is solved. THANK YOU!!

Collectives™ on Stack Overflow

SqlDataReader.Read() always false in C#

2 Answers 2

5 Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

5 Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related