mysql_num_rows in an if statement

Question

The problem I am facing is, mysql_num_rows gives me an output of 1 all through out the code, but when I match it wil 0 in an if statement, it returns true and does the code. so $license returns ........ instead of its actual value.

I tried to debug the problem myself using these.

Tried print_r to see if datas exists. - Yes.
Tried echoing the $license at first part - returns the right value.
Tried checking the value of mysql_num_rows - returns 1.
Matching it with 0 in an if statement - returns true when it should be false since the value is 1.

Any help on this?

$check = mysql_query("SELECT * FROM licenses WHERE email='$email'") or die(mysql_error
                                                                           ());
if (mysql_num_rows($check) > 0)
{
    while ($data = mysql_fetch_array($check))
    {
        print_r($data); // for test
        $name = $data['name'];
        $license = $data['pid'];
        echo $license; // test print 1
        $comments = $data['comments'];
    }

    if ($license == "Sgsmorgan")
        $license = "EWP Discounted Basic (Simpleleveraging)";
}

$count = mysql_num_rows($check); // for test
echo $count; // returns 1.
if (mysql_num_rows($check) == 0)
    $name = "";
$license = "...........";
echo $license;// test print 2
$comments = "Email doesnt exist in the database";

The mysql extension is outdated and on its way to deprecation. New code should use mysqli or PDO, both of which have important advantages, such as support for prepared statements. Speaking of, the sample code is potentially vulnerable to [SQL injection](http://unixwiz.net/techtips/sql-injection.html). Parameterize the statement to close the vulverability. — outis, Mar 25 '12 at 07:19
Don't use [`SELECT *`](http://stackoverflow.com/questions/321299/) unless you're writing a DB administration program; select only the columns you need. — outis, Mar 25 '12 at 07:20
For readability's sake, please pick and apply an [indent style](http://en.wikipedia.org/wiki/Indent_style). — outis, Mar 25 '12 at 07:21
In future, please consider indenting your code before posting it. It will make spotting errors much easier. — David Wolever, Mar 25 '12 at 07:23
I agree to both of your comments. I am just learning this up, so I thought I would write a basic frame which works and then get it fixed up against injections. About *, used it since that was easy than mentioning 3 fields, but will definitely do that. Still, I cant really find where my code goes wrong.. I have gone through the codes many times already. — Kishor, Mar 25 '12 at 07:23
@DavidWolever I have the code in indent style, but I just cant make it work here with putting the 4 spaces to get it as codes. Hitting the tab button doesnt work. — Kishor, Mar 25 '12 at 07:27
@Kishor To indent one level, use eight spaces, two levels use twelve spaces, etc. Alternately, write the code in your normal text editor, then copy+paste it in here (that's what I normally do). — David Wolever, Mar 25 '12 at 07:30
@DavidWolever - copy pasting from notepad didnt help, but I manually indented it. Please take a look. — Kishor, Mar 25 '12 at 07:36
@Kishor: to make indenting easy, you can use an editor or IDE that [auto-indents](http://www.hongkiat.com/blog/free-code-editors-reviewed/). For example, there's Notepad++ (MS Windows native only), Emacs/Xemacs, Vim, Eclipse, but don't limit yourself to this list. Notepad++ is lowest in terms of power & difficulty, Eclipse highest, and the others in-between. Emacs is what I used to indent your code sample. — outis, Mar 25 '12 at 18:20
@Kishor–RE: writing a basic system & fixing injection vulnerabilities later: if you do this, you're bound to miss some in any system that's complex enough to be useful. Start with parameterized statements and your code will be secure from the get-go. Even better, [separate](http://en.wikipedia.org/wiki/Separation_of_concerns) database access into its own module so the rest of the code doesn't depend on it. — outis, Mar 25 '12 at 18:45
@Kishor: note you can use markdown to create unordered lists with the "*", "-" and "+" characters, which is sometimes more suitable (and readable) than simply inserting line breaks. Click the orange question mark in the post editor for more markdown syntax. — outis, Mar 25 '12 at 18:48

score 3 · Accepted Answer · edited Mar 25 '12 at 18:42

3

Surely you mean this:

if (mysql_num_rows($check)==0)
{
    $name = "";
    $license = "...........";
    echo $license; //Test print 2
    $comments = "Email doesnt exist in the database";
}

Rather than

if (mysql_num_rows($check)==0)
$name = "";
$license = "...........";
echo $license; //Test print 2
$comments = "Email doesnt exist in the database";

Not using the curly brackets means only the first line below the if statement is included within it. So $license is always set to ............

Always use curly brackets.

edited Mar 25 '12 at 18:42

outis

75,655
22
151
221

answered Mar 25 '12 at 07:33

Michael

11,912
6
49
64

@Kishor - To confirm Michael's theory, what is the value that `$name` is set to at the end? – Anthony Mar 25 '12 at 07:47
That fixed it. I will keep that in mind, and wont repeat that stuff again :) Thanks – Kishor Mar 25 '12 at 07:50
1

@Kishor: I still on a weekly basis get errors from conditions like `if($x = 1)` instead of `if($x == 1)`, so I'm betting you'll repeat all kinds of stuff again. You'll just get better at hitting yourself on the head when it does. – Anthony Mar 25 '12 at 08:02
@Anthony, that's why it's worth doing `if (1 == $x)` as `if (1 = $x)` will result in a parse error. It's not as intuitive but it's less error-prone (or, rather, it's difficult to miss the errors if you have error reporting turned on). – Michael Mar 25 '12 at 08:04
@MichaelRushton - That's going to be a hard habit to learn, but I'll try it out. Thanks! – Anthony Mar 25 '12 at 08:12
1

@Anthony: in addition to [Yoda conditions](http://wiert.me/2010/05/25/yoda-conditions-from-stackoverflow-new-programming-jargon-you-coined/), another common idiom is to surround intentional assignments in conditions with additional parentheses: `if (isset($foo) && ($foo = trim($foo))) {...}`. Outside of Objective-C (and some [linters](http://stackoverflow.com/q/4250388/90527)), this won't generate any notices, but will inform any programmers not to change the assignment into an equivalence comparison. – outis Mar 25 '12 at 18:41

score 1 · Answer 2 · answered Mar 25 '12 at 07:26

1

I believe that the issues is that, at that point, there are no more rows left, as your while loop has fetched all of them.

If I'm not mistaken, this code:

while ($ignored = mysql_fetch_array($check)) {
    echo "Got a row! Rows left: " . mysql_num_rows($check);
}

Should output something like:

Got a row! Rows left: 3
Got a row! Rows left: 2
Got a row! Rows left: 1
Got a row! Rows left: 0

answered Mar 25 '12 at 07:26

David Wolever

148,955
89
346
502

There will be only 1 row where email='$email' I checked the value of mysql_num_rows as $count just before matching it with 0 in the if statement. $count says 0, the if statement gets true as well. – Kishor Mar 25 '12 at 07:30
@Kishor - Well, if it's not the while loop, then what happens if you change the condition to: `if ($count == 0)`? – Anthony Mar 25 '12 at 07:46
Same thing happens. well figured out it was me missing the curly brackets under the last if statement. -_- – Kishor Mar 25 '12 at 07:49

score 1 · Answer 3 · answered Mar 25 '12 at 07:36

1

Following up on David's root-cause, here is a really simple fix:

$check = mysql_query("SELECT * FROM licenses WHERE email='$email'") 
         or die(mysql_error());

if (mysql_num_rows($check) > 0) {
    while ($data = mysql_fetch_array($check)) {
        $name    = $data['name'];
        $license = $data['pid'];
        $comments = $data['comments'];
    }

    $license = ($license == "Blahblah") ? "This is a second level license" : $license;

} else {
    $name = "";
    $license = "...........";
    $comments = "Email doesnt exist in the database";
}

answered Mar 25 '12 at 07:36

Anthony

36,459
25
97
163

$license = ($license == "Blahblah") ? "This is a second level license" : $license; Seems a bit confusing as I am just learning php. Anyways it was me missing curly brackets under the last if statement. – Kishor Mar 25 '12 at 07:52
1

Yeah, part of why you missed it, I'm thinking, is because you have picked up the bad habit of not closing single-line conditionals in curly brackets, which is valid, but very hard to read and can quickly lead to losing track of where your other conditions open and close. Follow Michael's advice and always use curly braces. The reason I changed that one line (to conditional syntax known as [ternary](http://us3.php.net/manual/en/language.operators.comparison.php)) is because it often times helps clean up some of those spots where you just don't feel like having 3-5 lines of code to set 1 var. – Anthony Mar 25 '12 at 07:59

mysql_num_rows in an if statement

3 Answers3