Take the 2-minute tour ×
Stack Overflow is a question and answer site for professional and enthusiast programmers. It's 100% free, no registration required.

I have a SqlDB.dll that has the function:

         public SqlDataReader getEnumValues(int enumId)
    {
        SqlDataReader reader = null;
        using (SqlConnection connection = new SqlConnection(connectionString))
        {
            connection.Open();
            SqlCommand command =
                new SqlCommand(
                    "SELECT * FROM [EnumValue] WHERE enumId LIKE '" + enumId + "';",
                    connection);
            reader = command.ExecuteReader();
            //if(reader.Read())
            //    Debug.WriteLine("Inside sqlDb->getEnumValues command = " + command.CommandText + " reader[name] = " + reader["name"].ToString() + " reader[value] = " + reader["value"].ToString() + " reader[description] = " + reader["description"].ToString());
        }
        //reader.Close();
        return reader;
    }

As you can see I have tried closing the reader before returning it, and also I read the data inside and it's ok. I'm using the function like this:

 using (SqlDataReader getEnumValuesReader = (SqlDataReader)getEnumValues.Invoke(sqlDB, getEnumValuesForEnumParam))
                    {
                        Debug.WriteLine("Success getEnumValues -- ");

                        if (getEnumValuesReader.HasRows)
                        {
                            while (getEnumValuesReader.Read())        //Loop throw all enumValues and add them to current enum
                            {
                                try
                                {
                                    values.Add(new Model.EnumValue(getEnumValuesReader["name"].ToString(), getEnumValuesReader["value"].ToString(), getEnumValuesReader["description"].ToString()));
                                    Debug.WriteLine("Value[0].name = " + values[0].Name);
                                }
                                catch (Exception ex)
                                {
                                    Debug.WriteLine("Error in building new EnumValue: " + ex.Message);
                                }
                            }
                        }

                    }

and I'm getting exception of type 'System.InvalidOperationException'

I'm guessing it has something to do with the Sqldatareader being passed.

share|improve this question
    
SQL Injection alert - you should not concatenate together your SQL statements - use parametrized queries instead to avoid SQL injection –  marc_s Apr 20 at 20:32
    
I dont have a security problem at all, it does not have any user \ other developers \ plans for the future but thank you for the answer –  Yogevnn Apr 20 at 20:35
    
If this code is used for web sites - any of those web sites is in high danger of being exploited by SQL injection. Just let me know what web sites those are so I can make sure to never ever visit any of them .... –  marc_s Apr 20 at 20:37
    
No it is not for web, it is a VSPackage program I'm building. I'm familiar with the SQL injection problem but I'm not worried in this case. –  Yogevnn Apr 20 at 20:39
    
No matter what the end product is - it's just accepted best practice to avoid concatenating together your SQL. You should really use parametrized queries - ALWAYS, no exceptions. –  marc_s Apr 20 at 20:41

1 Answer 1

up vote 1 down vote accepted

The SQL reader cannot exist outside the context of the SQL connection. Your connection is being disposed in your method, so your returned reader cannot fetch any data in the calling method.

the best option is to return the values from the reader instead of the reader. It is not good practice to pass around readers, which means the underlying connection etc. Is open and undisposed.

share|improve this answer
    
Thank you for the answer, my problem is how to return all the values? because not all of the values are from the same type, some are strings and some are int –  Yogevnn Apr 20 at 20:24
    
you just need to declare a class with all your types (string, int etc.), and return a List<MyClass> from the method.. also called as DTOs. Data Transfer Objects. –  raja Apr 21 at 19:48

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.