Hit counter using mysql database

[jijimj73]

I am newbie as far as PHP is concerned. But I am thrilled. Could anyone tell me what's wrong with this code? Sometimes it shows errors.

<?php
if (isset($_GET['id'])) {
$id = $_GET['id'];
$curPageName = $curPageName. '-' .$id;
}

$sql = "SELECT counts FROM visitor_counter WHERE page_name = '$curPageName'";
$result = mysqli_query($conn, $sql);

$page_count = 0;
if ($result->num_rows > 0) {
    while($row = $result->fetch_assoc()) {
$page_count = $page_count + 1;
        $visits = $row["counts"] + 1;
  }
}

if ($page_count < 1) {
$sql = "INSERT INTO visitor_counter (page_name, counts) VALUES ('" .$curPageName. "', '1')";

if ($conn->query($sql) === TRUE) {
  $visits = "1";
} else {
  echo "";
}
}else{

$sql = "UPDATE visitor_counter SET counts = '$visits' WHERE page_name = '$curPageName'";
$result = mysqli_query($conn, $sql);
}

echo $visits. "  hits";
?>

 

[Barand]

What errors does it show? It would be helpful for us to know.

[mac_gyver]

1. if you got an error, you need to post it.
2.  the $page_count variable doesn't exist, likely causing an undefined variable error, nor is it used anywhere in the posted code.
3. you don't need to SELECT data in order to decide to insert or update it. just use an INSERT ... ON DUPLICATE KEY UPDATE ... query.
4. you can use the MySql LAST_INSERT_ID(expr) function in the query to make the counts column value accessible via the expr (expression) argument without needing to execute another query.
5. don't put external, unknown, dynamic values directly into an sql query statement. use a prepared query instead.
6. if it seems like using a prepared query with the mysqli extension, this would be a good time to switch to the much simpler and more modern PDO extension.

[maxxd]

In addition to that, you're using $curPageName all over that code, but I don't see where it's defined outside the if conditional. This means that if $_GET['id'] is not set, neither is $curPageName.

Edited January 12 by maxxd

[Phi11W]

11 hours ago, mac_gyver said:

you don't need to SELECT data in order to decide to insert or update it. just use an INSERT ... ON DUPLICATE KEY UPDATE ... query.

I would go further and say you must not "select data in order to decide to insert or update it".  That road leads to Race Conditions. 

 

Whist "on duplicate update" exists and works well, I would suggest making a conscious decision about which is the more likely condition to occur. 
In this case, I would say that updates are far more likely that inserts (with new pages only being added occasionally) so I would code the update first, and check to see whether zero rows were updated by it and, if so, insert the new row. 

Regards, 
   Phill Ward. 

[Andou]

Whats error are you getting?