SqlDataReader isn't working, it skips the part where it checks the username

Question

I am working on a project that has 1 client and 1 server. I created also a Class library in which I put the code and than ad this class library as Reference at the Server and at the Client. And so I call methods from the Client by creating an object of the class. And it is working very good... until now!

In the class library i have method called Check(), which checks if the Username already exist (when trying to register, and some other stuff. But the problem is the step in which it checks the username from the database is Skipped!! I don't know why, it doesn't show me an error why. The only error that comes up is the error that appears when trying to insert the same username into the database due to that skipped step.

Here is the Class library code:

    bool busy = false;
    bool empty = false;
    bool notSame = false;
    bool completed = false;

public void Check(string a1, string b2, string c3, string d4, string e5, string f6)
    {
        SqlConnection con = new SqlConnection(@"Data Source=TALY-PC;Initial Catalog=TicTacToe;Integrated Security=True");

        SqlCommand cmd = new SqlCommand("SELECT * FROM tblUsers", con);

        if (con.State.Equals(ConnectionState.Open))
        {
            return;
        }
        else
        {
            con.Open();
        }

        SqlDataReader dr = cmd.ExecuteReader();

        while (dr.Read())
        {

            if (dr["Username"].ToString() == d4)
            {
                busy = true;
            }
            else if (a1 == "" || b2 == "" || c3 == "" || d4 == "" || e5 == "" || f6 == "")
            {

                empty = true;

            }
            else if (e5 != f6)
            {
                notSame = true;
                return;
            }
            else
            {
                dr.Close();
                SqlCommand cmd1 = new SqlCommand("INSERT INTO tblUsers (Name, LastName, email, Username, Password) VALUES ('" + a1 + "', '" + b2 + "', '" + c3 + "', '" + d4 + "', '" + e5 + "')", con);

                cmd1.ExecuteNonQuery();

                completed = true;
            }
        }

And the skipped part is this part:

if (dr["Username"].ToString() == d4)
            {
                busy = true;
            }

I don't know why it doesn't check this part??

Here is the code from the Client:

HttpChannel chan = new HttpChannel();

    Tic obj = (Tic)Activator.GetObject(typeof(Tic), "http://127.0.0.1:9050/MyMathServer");

private void RegisterForm_Load(object sender, EventArgs e)
    {
        ChannelServices.RegisterChannel(chan);
    }

 private void Button1_Click(object sender, EventArgs e)
   {
 obj.Check(textBoxX1.Text, textBoxX2.Text, textBoxX3.Text, textBoxX4.Text, textBoxX5.Text, textBoxX6.Text);
        label8.Text = obj.getUseri();
        if (obj.getBusy() == true)
        {
            MessageBox.Show("This username is already Taken, please choose another username");

            obj.setBusy();

        }
        else if (obj.getEmpty() == true)
        {
            MessageBox.Show("Please fill all the fields!");
            obj.setEmpty();
        }
        else if (obj.getNotSame() == true)
        {
            MessageBox.Show("Passwords don't match!!");
            textBoxX5.Text = "";
            textBoxX6.Text = "";
            obj.setNotSame();
        }
        else if (obj.getCompleted() == true)
        {
            MessageBox.Show("Registration was completed successfully. Please close the window and Log Int ");
            textBoxX1.Text = "";
            textBoxX2.Text = "";
            textBoxX3.Text = "";
            textBoxX4.Text = "";
            textBoxX5.Text = "";
            textBoxX6.Text = "";
            obj.setCompleted();

        }
  }

Can someone please tell me why isn't it checking the username?

Answer 1

I do not exactly understand your code. I can make some simple observations:

• First of all: use using of your connection and datareader. Whatever happens (exceptions, etc.), using will close them.
• Use better names for your variables. a1 till f6 mean nothing. Giving them meaningfull names makes it easier for us to read... so... If realy have no idea why e5 must be equal to f6.
• Same thing goes for your textboxes.
• The statement if (a1 == "" || b2 == "" || c3 == "" || d4 == "" || e5 == "" || f6 == """) is inside the while-loop, but since those values never change, your might consider moving them out of the loop.
• The same thing goes for the line if (e5 != f6).
• You check if the connection is open. But this connection has just been created. How can it be open? And if it would be open... it would simply return without doing anything?
• If you reach condition (e5 != f6) and the condition is true, your return. But you do so without closing the connection.
• If all parameters are "", but there are no records in the database, empty will not be set. Is this by design?

Try this code:

public enum Result
{
    Busy,
    Empty,
    NotSame,
    Completed
}

public Result Check(string name, string lastName, string eMail, string userName, string password1, string password2)
{
    // You should check with String.IsNullOrEmpty(...), not just an empty string.
    if (name == "" || lastName == "" || eMail == "" || userName == "" || password1 == "" || password2 == "")
        return Result.Empty;

    if (password1 != password2)
        return Result.NotSame;

    using(SqlConnection con = new SqlConnection(@"Data Source=TALY-PC;Initial Catalog=TicTacToe;Integrated Security=True"))
    {
        SqlCommand cmd = new SqlCommand("SELECT COUNT(*) FROM tblUsers WHERE UserName=@0", con);
        cmd.Parameters.AddWithValue("@0", userName);
        int count = (int)cmd.ExecuteScalar();
        if (count > 0)
            return Result.Busy;

        // I must admit, also the insert values should be done with SqlParameters.
        cmd = new SqlCommand("INSERT INTO tblUsers (Name, LastName, email, Username, Password) VALUES ('" + name + "', '" + lastName + "', '" + eMail + "', '" + userName + "', '" + password1 + "')", con);
        cmd.ExecuteNonQuery();
        return Result.Completed;
    }
}