jijimj73 Posted January 11 Share Posted January 11 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"; ?> Quote Link to comment Share on other sites More sharing options...
Barand Posted January 11 Share Posted January 11 What errors does it show? It would be helpful for us to know. Quote Link to comment Share on other sites More sharing options...
mac_gyver Posted January 11 Share Posted January 11 if you got an error, you need to post it. the $page_count variable doesn't exist, likely causing an undefined variable error, nor is it used anywhere in the posted code. 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. 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. don't put external, unknown, dynamic values directly into an sql query statement. use a prepared query instead. 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. Quote Link to comment Share on other sites More sharing options...
maxxd Posted January 12 Share Posted January 12 (edited) 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 Quote Link to comment Share on other sites More sharing options...
Phi11W Posted January 12 Share Posted January 12 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. Quote Link to comment Share on other sites More sharing options...
Andou Posted January 19 Share Posted January 19 Whats error are you getting? Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.