Is isset() here right? / or is my if-condition wrong?

Question

i am making a Tube-Site and I can´t finish my code. At the moment I am working on a Favorize-Script, so registered people can "favorize" videos and can see them on a special site.

Logically it should not be possible to favorize a video twice and that´s my problem at the moment.

But I think my if-condition is wrong because it doesn´t check if the userid and the videoid exist!

Here is my code:

<?php
 session_start();
 include('config.php');

    $videoid = $_GET['id'];
    $userid = $_SESSION["username"];

    $result = mysql_query("SELECT `id` FROM `users` WHERE `username` = '$userid'") or die (mysql_error());
    $row = mysql_fetch_assoc($result);
    $usernameid = $row['id'];

    if (isset($userid, $videoid))
    {
    $row = "INSERT INTO favorites (userid, videoid, time)
    VALUES
    ('$usernameid','$videoid', now())";

    mysql_query("$row");

    header ("Location: http://localhost/video.php?id=" . $_GET['id'] . ""); 
    } 
    else
    {
    header ("Location: http://localhost/video.php?id=" . $_GET['id'] . "");
    }


?>

My Favorites-Table: favoriteid | userid | videoid | time

So it should check everytime if a video and a username is already set.

I also tried !isset but it doesn´t works

`isset()`'s **ONLY** purpose is to check for the mere existence of a variable. It does not care if the variable is empty, blank, or if it is a 10000-item array. — MonkeyZeus, Dec 11 '13 at 19:23
I would use `isset()` to check if your index exists in the $_GET or $_SESSION array, because if they are not set , you will get an error that the index are not exists. — demonking, Dec 11 '13 at 19:25
`isset()` is still plenty useful and you should leave it in but you need to also check if the two variables, `$userid` and `$videoid`, are not nothing — MonkeyZeus, Dec 11 '13 at 19:36
@monkey but $userid and $videoid are inserting. the problem is that the if condition doesnt check if they exist. so everytime its inserting new rows. do i really need that and if yes how can i make that? — user3092548, Dec 11 '13 at 19:44
Your use of `isset()` is doing nothing to check against the existence in the database. `if (isset($userid, $videoid))` will always work because you are setting the variables in lines 5 and 6 `$videoid = $_GET['id']; $userid = $_SESSION["username"];` so you are guaranteeing that they are always set. Comment out lines 5 and 6 and then the insert will no longer happen. Also, you are checking for `$userid` in the `if()` but the insert is providing `$usernameid`? Also, I am purposely not giving you the exact code to fix this because I want you to learn :) — MonkeyZeus, Dec 11 '13 at 19:53
Your logic should be like this: `Get username from session and videoid from $_GET --> Make sure videoid is not nothing --> use SQL to check the favorize table if there is a record with that userid and videoid combo --> count the rows returned by mysql --> if rows returned by mysql < 1 then proceed with insert` — MonkeyZeus, Dec 11 '13 at 20:00
hmm, i am a bit confused.. so at first, all variables give me the right numbers. i dont get this with the sql.. and the rest after sql should i make a new columns with "favorited" or somtehing else.. (boolean) and inserts as true.. — user3092548, Dec 11 '13 at 20:09
Nope, you already have a `favorites` table so you should check if the userid and videoid combo exist in there before inserting or you can set up something called a [Composite Primary Key](http://stackoverflow.com/questions/5835978/how-to-properly-create-composite-primary-keys-mysql) so that the INSERT will simply fail next time it tries and you can completely remove the `if(isset())` logic — MonkeyZeus, Dec 11 '13 at 20:32

MikeF · Answer 1 · 2013-12-11T20:58:00.900

0

UPDATE

if($userid != '' && $videoid != ''){
    $check = mysql_query("SELECT * FROM `favorites` WHERE `userid` = '$usernameid' AND `videoid` = '$videoid'");

    if (mysql_num_rows($check) == 0) {
        $row = "INSERT INTO favorites (userid, videoid, time)
        VALUES ('$usernameid','$videoid', now())";

        mysql_query("$row");

        header ("Location: http://localhost/video.php?id=" . $_GET['id'] . ""); 
    }else{
        echo "You've already voted!";
    }
}else{
    header("Location: http://localhost/");
}

edited Dec 11 '13 at 20:58

answered Dec 11 '13 at 19:23

MikeF

485
6
22

What that's doing is checking whether both variables are empty. If they're not empty, then it proceeds with the if statement, if they're empty then it goes to the else statement. You should change the URL of your else statement header for debugging purposes. Maybe put it to the home page, as currently both of your if & else statements go to the same url. – MikeF Dec 11 '13 at 19:36
@user3092548 try the update, see if that has any impact on the result – MikeF Dec 11 '13 at 19:39
yeah i commented the else but still not working. its always inserting the variables – user3092548 Dec 11 '13 at 19:40
you can try `if((trim($userid) != '') && (trim($videoid) != ''))` maybe there is something like a whitespace. – demonking Dec 11 '13 at 19:43
@user3092548 - If you echo the $userid & $videoid before the if statement, what are they outputting? – MikeF Dec 11 '13 at 20:00
$userid gives me the username, $videoid the gives me the id of the video and usernameid the id of the user – user3092548 Dec 11 '13 at 20:06
If you try the following: `if($userid == '' && $videoid == ''){ die("Empty"); } else { die("Not empty"); }` - What does it output? – MikeF Dec 11 '13 at 20:08
It gives me "Not empty" – user3092548 Dec 11 '13 at 20:14
@user3092548 - Ok, try my update. I've removed the parenthesis from the if statement – MikeF Dec 11 '13 at 20:16
@user3092548 - Ok, can I just get you to confirm that the if statement is working and checking that the 2 variables are populated? Now the issue is that it needs to check whether they've already voted? – MikeF Dec 11 '13 at 20:35
fixed it :D <3 look at the code. it could be possible that i used your method but at first i didnt understand it – user3092548 Dec 11 '13 at 21:02

MrHunter · Answer 2 · 2013-12-11T19:32:53.450

You should consider looking into MySQLI since mysql_* functions are slowly being deprecated.

Anyhow you should use a function called mysql_num_rows and check if the result is either 0 or 1. If the returning result is 1 you will know the video is already favourited and shouldn't repeat the action. If you get a 0 then insert it!

if (mysql_num_rows($result) == 0 && isset($userid) && isset($videoid)) {
$row = "INSERT INTO favorites (userid, videoid, time)
VALUES ('$usernameid','$videoid', now())";

    mysql_query("$row");

    header ("Location: http://localhost/video.php?id=" . $_GET['id'] . ""); 
}

i tried to seperate them but it gave me a "null !== expression" — user3092548, Dec 11 '13 at 19:34

score -2 · Answer 3 · answered Dec 11 '13 at 19:23

-2

Change:

if (isset($userid, $videoid))

for:

if (isset($userid) && isset($videoid))

answered Dec 11 '13 at 19:23

Leonardo

736
4
11

Please reference the [manual](http://www.php.net/isset) because `var_dump(isset($a, $b)); // TRUE` is in there. – MonkeyZeus Dec 11 '13 at 19:25

Is isset() here right? / or is my if-condition wrong?

3 Answers3

UPDATE