1

i have a little problem. I need to Hide one button if my sql request is true. I want to hide admin panel button for users, who are not admins. I have one row in table that is "Type", and it have '1' or '2' int numbers. If is 1 this is admin, if it's 2 is user. I am making mistake somewhere. //

            string myConnection = @"Data Source=.....";
            SqlConnection myConn = new SqlConnection(myConnection);
            **SqlCommand SelectCommand = new SqlCommand("Select * from dbo.Admin where Type=2 and UserName='" + l1.UserName.Text + "';", myConn);**
            SqlDataReader myReader;
            myConn.Open();
            int i = SelectCommand.ExecuteNonQuery();
            if (i > 0)
            { 
                **btnAdminPanel.Hide();**
            }
BlackStock
  • 11
  • 4

2 Answers2

1

Calling ExecuteNonQuery for a SELECT statement is totally wrong. This method should be used when you expect to know how many rows are affected by an INSERT/UPDATE/DELETE statement (or other Data Definition Language queries)

You should use ExecuteReader and check if the reader HasRows == True.
But if you are just interested to know if that particular user with the specified type exists then you can use a different query text that could be run by ExecuteScalar (returns a single row/column value)

string cmdText = @"IF EXISTS(Select 1 from dbo.Admin 
                             where Type=2 and UserName=@user)
                             SELECT 1 ELSE SELECT 0";
using(SqlConnection myConn = new SqlConnection(myConnection))
using(SqlCommand SelectCommand = new SqlCommand(cmdText, myConn))
{
    myConn.Open();
    SelectCommand.Parameters.Add("@user", SqlDbType.NVarChar).Value = l1.UserName.Text;
    int i = (int)SelectCommand.ExecuteScalar();
    if (i > 0)
    { 
        btnAdminPanel.Hide();
    }
}

This query returns 1 if there is a record that satisfies the condition or zero if there is no record. It is better because the database will execute this query knowing that you are interested only in the existence of the record and not on the content of the record. From the client side this is better because the NET library doesn't need to build an SqlDataReader to read an unknown number of records and you don't need to check for null return values.

Also note that I have used parameters to pass the query values. Never use string concatenations if you want to avoid hackers throw a party with your code. Sql Iniection is based on this string concatenation approach.

Finally disposable objects go inside a using block to avoid resources leaks.

Steve
  • 213,761
  • 22
  • 232
  • 286
  • Yo, your answer is great, i like it, i tried it now, but it's not working by the way that i want, to hide this button btnAdminPanel. There is no errors, but i cant understand why this is not working cuz it's need to be work in theory. – BlackStock Mar 27 '17 at 22:11
  • Sorry, but I can't see any problem here. And it is late now, so goodnight. – Steve Mar 27 '17 at 22:24
  • 1
    I am sorry, thank you very much for support and the code. My mistake has come from that i dont recieve username.text from loginForm to Form1 properly. It was little mistake. TY again, and i marked it true answer! – BlackStock Mar 27 '17 at 23:00
0

I think you should use ExecuteScalar instead of ExecuteNonQuery. Also, you can find here the differences between the diffrent execution methods provided for the SqlCommand.

Community
  • 1
  • 1
Reda Ramadan
  • 31
  • 1
  • 3