C# to SQL ExecuteReader() getting stuck

Question

When running the below method in my application the app freezes and when I pause VS it seems to be stuck on the line that goes:

SqlDataReader reader = select.ExecuteReader();

I've got other SQL methods running fine so I know the connection string is correct, I've double checked the SQL and that's fine. Am I wrong in think the reader variable can not contain the returning value of the scalar function when the ExecuteReader() is called?

public static bool AccountValidation(string username, string password)
{
    string statement = "select dbo.AccountValidation('" + username + "','" + password + "')";
    SqlCommand select = new SqlCommand(statement, connect);
    connect.Open();
    SqlDataReader reader = select.ExecuteReader();
    string result = reader.ToString();
    connect.Close();

    if (result != "true")
    {
        return false;
    }
    else
    {
        return true;
    }
}

Answer 1

The main problem is that you are not actually reading anything back from the data reader, you have to iterate over the result set and then read based on ordinal/positional index.

There are also other big problems like

• Not using parameters which leaves the code vulnerable to sql injection attacks.
• Not wrapping your disposables in using blocks which could leave database connections open if there are exceptions
• sharing db connections in your types. Create connections on an as needed basis and then dispose them when you are done.

Here is your updated code with fixes. I guessed at the column types (varchar), fix that and the lengths as they are implemented in your schema.

public static bool AccountValidation(string username, string password)
{
    const string statement = "select dbo.AccountValidation(@username, @password)";
    string result = null;

    // reference assembly System.Configuration
    string connStr = System.Configuration.ConfigurationManager.ConnectionStrings["YourDb"].ConnectionString;

    using(var connection = new SqlConnection(connStr))
    using(SqlCommand cmd = new SqlCommand(statement, connect))
    {
        cmd.Parameters.Add(new SqlParameter("@username", SqlDbType.VarChar, 200){Value = username});
        cmd.Parameters.Add(new SqlParameter("@password", SqlDbType.VarChar, 200){Value = password});
        connect.Open();
        using(SqlDataReader reader = cmd.ExecuteReader())
        {
            if(reader.Read())
                result = reader.GetString(0); // read back the first column of the first row
        }
    }
    if (result != "true")
    {
        return false;
    }
    else
    {
        return true;
    }
}

---

On a side note it would be cleaner to return a bit from your database function AccountValidation and then read that back with reader.GetBoolean(0) and assign that to the result and return that directly instead of doing string comparisons.

Also, as mentioned above in the comments, if you are only returning 1 value it is easier (and less code) to call ExecuteScalar instead of ExecuteReader.