CODEWITHSUNDEEP
c#sql
Tom Crook
Tom Crook

Reputation: 11

'else' of if/else not executing when it should C#

I am designing a ATM system in C# and for the login function I am using an SQL server database to compare the card number and PIN entered to those on in the database. When the correct card number and PIN are entered everything works fine but when a incorrect value it entered nothing happens at all and I have been trying to figure it out for some time now. Does anyone have any ideas to what it might be? Even my university lecturer has no idea!

//Select all fields from the table 'ATMCards' using the connection previously created and use the SqlDataReader to read the values

To simplify it I have just put the messagebox.show in the else as all I want to do is at least have it trigger!

SqlCommand cmd = new SqlCommand("SELECT * FROM [ATMCards] WHERE (cardNumber = @cardNumber) AND (PIN = @PIN)", cn);
cmd.Parameters.AddWithValue("@cardNumber", cboxSimCard.Text);
cmd.Parameters.AddWithValue("@PIN", txtboxPIN.Text);

cmd.Connection = cn;
SqlDataReader r = null;
r = cmd.ExecuteReader();

//While the reader is in execution:
while (r.Read())
{
    //ADD IF NOT CONFISCATED DO THIS:
    if (((Boolean)(r["confiscated"]) == notConfiscated))
    {
        string cNum =  r["cardNumber"].ToString();
        string pin = r["PIN"].ToString();

        //Compare the results in the ATMCards table against those on the form used to log in
        if (string.Equals(cNum,cboxSimCard.Text) && string.Equals(pin,txtboxPIN.Text))
        {
            MessageBox.Show("Card number "+cNum+"  PIN "+pin); 
            //If the login details are correct then grant access to the menu screen by creating a new instance of it and hide the login form. Clear PIN to avoid the next user accessing the account
            MessageBox.Show("Open form all is good"); 
            txtboxPIN.Clear();
            Form myNewForm = new Menu();
            myNewForm.Show();
            this.Hide();
            break;
        }

        else
        {
            MessageBox.Show("Here"); 
        }

Upvotes: 0

Views: 305

Answers (4)

Yahia Alhami
Yahia Alhami

Reputation: 1

Man your code is

  1. Not secure and maybe injected
  2. Credit card and pin must be encrypted and u didnt
  3. Your code will never show any message in failure case since if no data is reutrned the if clause inside while will not executed

you can use 

if(reader.HasRows)
///then do happy case
else
//show error message

Upvotes: 0

Jodrell
Jodrell

Reputation: 35706

When the there is no match for your where conditions the result set is returned with no rows. The first r.Read returns false and the contents of the while loop is never executed.

As Henk Holterman comments, if you debug the code this will be obvious.

Further to the simple answer ...

You shouldn't be executing your SQL like this. It leaves you open to injection attacks. You could use Linq-To-Entities or wrap you SQL in a call to sp_ExecuteSQL.

The PIN shouldn't be stored as plain text but rather as a secure hash of the PIN and other card details.

The card number shouldn't be stored in plain text. If you never have to read the card number is could be hashed. Otherwise encrypted

I assume you are validating the card details, using a modulo-13 or some industry standard check before passing the query to the database.

I'm not suggeting you could recreate what actually happens inside an ATM but, some nod to the security required with sensitive data would surely yield extra credit. (pun not intended)

Upvotes: 2

Tigran
Tigran

Reputation: 62248

To me it seems that this :

if (string.Equals(cNum,cboxSimCard.Text) && string.Equals(pin,txtboxPIN.Text))

Does somethign wrong. Try to use an overload of the same function with String.Equals(String, StringComparison). Like this 

if (cNum.Equals(cboxSimCard.Text, StringComparison.InvariantCultureIgnoreCase) &&   pin.Equals(txtboxPIN.Text, StringComparison.InvariantCultureIgnoreCase))

Upvotes: 0

Moo-Juice
Moo-Juice

Reputation: 38825

When an incorrect card number and pin pair are entered, no rows are returned from the database, so the while(r.Read()) returns false immediately.

Upvotes: 8

Related Questions