Hunt3r
Hunt3r

Reputation: 65

PHP/MySQL: Not inserting into database

Alright, I'm trying to make it so that someone can vote for a "server" every 24 hours, but currently, I'm stuck here:

    function vote1() {
    $pull = $_SERVER['REMOTE_ADDR'];
    $votersIp = "select votersIp from voters";
    $usersIp = $_SERVER['REMOTE_ADDR'];





    $fetch = mysql_query("SELECT * FROM voters WHERE votersIp = '".$_SERVER['REMOTE_ADDR']."'");



        while($rude = mysql_fetch_array($fetch)){  

    if($rude[votersIp] != $pull) {
    $zoot="INSERT INTO voters (votersIp, lastVoted) VALUES ('$usersIp', '0')";
    mysql_query($zoot) or die ( mysql_error() );
    echo 'Voters IP not in database'; //debugging
    }

    if($rude[votersIp] = $pull) {
     echo 'found ip'; //debugging
    }

}

So, if you can't tell, I'm trying to make it so that everytime someone votes, it adds their IP address to the database. I know I'm doing something very basic wrong here, but I'm still learning.

I would appreciate it if you could post the fix, but also explain what I'm doing wrong, and any suggestions would be great as well.

Thank you!

Upvotes: 0

Views: 512

Answers (4)

J. Bruni
J. Bruni

Reputation: 20492

There are several problems in your code.

  1. A close bracket } is missing, either for the while or for the function

  2. We don't need to loop through all the voters table to find out if an IP is in the database or not. We can simply use a WHERE clause in the SQL command, like this: SELECT votersIp FROM voters WHERE votersIp = $userIp

  3. There are a lot of unnecessary variables with strange names.

  4. As a security measure, anything we will use as a part of a SQL command should be escaped, to avoid being vulnerable to some types of attacks.

  5. When array indexes are strings, they should be between quotes. So, $rude[votersIp] is wrong, while $rude['votersIp'] is correct. (Without quotes, PHP interprets it as a constant, instead of a string.)

  6. The $rude[votersIp] = $pull statement is a variable assignment. You should use the correct operator == to compare values: $rude[votersIp] == $pull

  7. Bad indentation makes the code harder to be understood for us, human readers. Good indentation is better.

  8. The while loop is done upon the results of the query which searchs for a specific IP. This means the first condition of the if inside it will NEVER be true (a different IP), so never the INSERT will run.

Here is an improved version:

function vote()
{
    $ip = $_SERVER['REMOTE_ADDR'];
    $select = 'SELECT votersIp FROM voters WHERE votersIp = "' . mysql_real_escape_string($ip) . '"';
    $search = mysql_query($select);
    $voter = mysql_fetch_array($search);

    if (!empty($voter))
    {
        echo 'found ip'; //debugging
    }
    else
    {
        echo 'Voters IP not in database'; //debugging
        $insert = 'INSERT INTO voters (votersIp, lastVoted) VALUES ("' . mysql_real_escape_string($ip) . '", "0")';
        mysql_query($insert);
    }
}

Suggestion: you should read and study a good book about programming for beginners. As I can't recommend a specific book for you, I suggest you ask for a book recommendation. A good book to learn how to program, for beginners.

Upvotes: 4

Dan Kanze
Dan Kanze

Reputation: 18595

Use this:

function vote1() {

    $mysqli=new mysqli("hostname", "username", "password", "database");

    //Check to see if voter is in DB
    $stmt = $mysqli->prepare("SELECT id FROM voters WHERE votersIp = ?");
    $stmt->bind_param('s', $_SERVER['REMOTE_ADDR']);
    $stmt->execute();
    $stmt->bind_result($rude);
    $stmt->fetch();
    $stmt->close();

    //If not, add him to the DB
    if($rude[votersIp] != $_SERVER['REMOTE_ADDR']) {

        //The DateTime
        $mySqlDateTime= date("Y-m-d H:i:s", $_SERVER['REQUEST_TIME']);

        //Insert Into DB
        $stmt = $mysqli->prepare("INSERT INTO voters (votersIp, lastVoted) VALUES (?, ?,)");
        $stmt->bind_param('ss', $_SERVER['REMOTE_ADDR'], $mySqlDateTime);
        $stmt->execute();
        $stmt->close();

        echo "User Added";
    }

    else{ echo "User Already Exhists"; }
}

Upvotes: 2

Marc B
Marc B

Reputation: 360572

Your logic needs improvement - you do a query to fetch ALL records with a particular IP in them, then loop over those results, and insert a record if the very ip you just told the database to fetch for you is NOT in the results.

That's like going into a restaurant, ordering a steak, then complaining to the waiter that you were served steak (IP is in the database). If the restaurant is out of steak (IP is not listed), your code does nothing and simply ignores the vote, because you only record the IP if it's alread in the database.

You should have something like this:

$fetch = mysql_query("SELECT count(*) AS cnt FROM voters WHERE votersIp='{$_SERVER['REMOTE_ADDR']}'") or die(mysql_error());
$row = mysql_fetch_assoc($fetch);
if ($row['cnt'] == 0) {
   // ip is not in the database, allow the vote
} else {
   // ip is listed, no vote for you!
}

Upvotes: 3

BenOfTheNorth
BenOfTheNorth

Reputation: 2872

You haven't mentioned whats actually going wrong, but you have a problem here:

if($rude[votersIp] = $pull) {

Here, you are assigning $rude[votersIp] the value of $pull instead of comparing it - it should read:

if($rude[votersIp] == $pull) {

Upvotes: 1

Related Questions