PHP/MYSQL remove row by id

[iConqueror]

I have a form that asks the user to enter in an exam id that they want to delete from the exam table in the DB.

The problem I am having is that even if the user enters in an id that does not match in the DB the script fires the else block completely ignoring the condition.

I may have implemented this wrong but as far as I am aware of i cannot see where I am wrong but I suspect that there is something I have done wrong in my if statment.

Here is my form that requires the user to enter an exam id for deletion

<form method = "post" action = "examDeleted.php">
    <h1 class = "title">Delete Exam</h1>
    <div class = "formContent">
        <labeL for = "id">ID</labeL>
        <input type = text name = "id" class = "input">
        <br><br>
        <br><br><br>
        <input type = "submit" value = "Delete">
    </div>
</form>`

This is passed to the function that contains the sql query to delete the record from the table

<?php
include('dbLogin.php');
$id = trim($_POST['id']);

if($id != "")
{
    $delete = "DELETE FROM exams WHERE id = '$id'";
    $results = mysql_query($delete);

    if(!$results)
    {
        die ("Cannot delete data from the database! " + mysql_error());
        echo '<br><br>';
        echo '<a href="home.html">Return</a>';
    }
    else
    {
        echo"Exam: $id has been deleted";
    }
}
else
{
    echo "No data entered! " . mysql_error();
}

?>

As you can see the condition !$results, to me it is saying if the record does not exist then kill the query else confirm the deletion. Is there an obvious reason why the inner if statment dosent get fired?

[cereal]

Hi, dont' use + to concatenate strings like in javascript, use dots:

die ("Cannot delete data from the database! " . mysql_error());

[gabrielcastillo]

for security reasons you should check if your id post is numeric.

if( is_numeric($id) )

This will only allow numeric values to hit your database query.

Also, use mysql_affected_rows() to check if your query has ran.

if(mysql_affected_rows() > 0){
    //Run Success Notice
}else{
    //Run Failed Notice
}

[cereal]

To the downvoter, please explain me why.

Reading this:

As you can see the condition !$results, to me it is saying if the record does not exist then kill the query else confirm the deletion. Is there an obvious reason why the inner if statment dosent get fired?

I understand that this:

if(!$results)
{
    die ("Cannot delete data from the database! " + mysql_error());
    echo '<br><br>';
    echo '<a href="home.html">Return</a>';
}

Doesn't return anything and the reason is the + sign, example:

$a = 'hello';
$b = 'world';

echo $a + $b;

Returns blank, with var_dump() returns int(0), if you write:

die($a + $b);

It will still return blank, unless $a and $b are integers or numbers enclosed by quotes:

$a = '1'; # $a = 1;
$b = '2'; # $b = 2;

If I'm missing something, please explain. Bye :)

[veedeoo]

@Cereal

I don't understand why you get a down vote for it though. It must be a C++ pride or something. I will give you an up vote to reflect 0 vote :).

[rubberman]

If the if($id != ""), try !== instead of != - this is one of the subtle issues that trip up a lot of PHP programmers. See the PHP online documentation about this.

A few other pointers that may prevent you having to come back...

include('dbLogin.php');
$id = trim($_POST['id']);

Suggests that $_POST['id'] is always set. You should make provisions for when this is not true as in direct access via url or curl or form spoofing. Place a redirect as a condition for this case...

if(isset($_POST['id']))
{
    //check for integer - but be careful which function(s) you use, e.g.
    //do you want to accept a string (e.g. itneer with a trailing space)
    //if conditions met...

    include('dbLogin.php'); //no need to include it outside the conditional blocks if it's not being used outside the conditional blocks 
    //...

}else{
    //redirect safely
}

In addition, you seem to be happy that an user can delete any record from the DB, whether that record "belongs" to them or not...

DELETE FROM exams WHERE id = '$id'

Ideally you'd have a session "userid" that would be passed to the query...

DELETE FROM exams WHERE id = '$id' AND user_id = '$userid'

Based on the number of affected rows (result), you'd return true or false to the user.

It should be noted that you are using mysql_* functions. Please consider using mysqli or PDO and the prepared statement object...e.g.

$stmt = $db->prepare("DELETE FROM exams WHERE id = ':id' AND user_id = ':userid'");

And then bind the parameters accordingly.