1st_Edition_Charizard Posted December 15, 2014 Share Posted December 15, 2014 (edited) Hello all, im new to this forum, im a noobie, just started coding about 3 weeks ago, don't be too hard on me. My question is I wan't to check for challenges a user's team posted that have not been excepted after 1 day, then auto refund the user's team back there credits, and then also delete all the challenges. So far here is my code. //Delete all matches not accepted after 1 day $arrayin = array(); $autorefund = mysql_query("SELECT * FROM `challenges` WHERE `a` = " . $team['id'] . " " . "AND `accepted` = 0 AND `completed` = 0 AND `chtype` = 1 AND (`expires` < " . ((int) time()) . ")"); if (mysql_num_rows($autorefund) > 0) { while ($autorefund = mysql_fetch_assoc($autorefund)) { $arrayin[] = $autorefund['id']; mysql_query("UPDATE `teams` SET `balance` = `balance` + " . $autorefund['credits'] . " " . "WHERE `id` IN (" . mysql_real_escape_string(implode(',', $arrayin)) . ")"); mysql_query("DELETE FROM `challenges` WHERE `a` IN " . "(" . mysql_real_escape_string(implode(',', $arrayin)) . ") " . "AND `accepted` = 0 AND `completed` = 0 AND `chtype` = 1 " . "AND (`expires` < " . ((int) time()) . ")"); } } FYI here is the code i had, it works fine, however it will only delete 1 challenge per team, not all of there challenges. $autorefund = mysql_query("SELECT * FROM `challenges` WHERE `a` = " . $team['id'] . " AND `accepted` = 0 AND `completed` = 0 AND `chtype` = 1 AND (`expires` < ".((int)time()).")"); if (mysql_num_rows($autorefund) > 0) { while ($autorefund = mysql_fetch_assoc($autorefund)) { if ($autorefund['accepted'] == 0 and $autorefund['expires'] < time()) { mysql_query("UPDATE `teams` SET `balance` = `balance` + " . $autorefund['credits'] . " WHERE `id` = " . $team['id'] . ""); mysql_query("DELETE FROM `challenges` WHERE `a` = " . $team['id'] . " AND `accepted` = 0 AND `completed` = 0 AND `chtype` = 1 AND (`expires` < ".((int)time()).")); } } } I inherited this script, its using deprecated statements, i know im not skilled to rewrite the entire site to use prepared statements. Edited December 15, 2014 by mac_gyver code tags around posted code please Quote Link to comment Share on other sites More sharing options...
Solution mac_gyver Posted December 15, 2014 Solution Share Posted December 15, 2014 the reason your code only works for one row is because you are reusing and overwriting the $autorefund variable, in both sets of code you posted. if you had php's error_reporting set to E_ALL and display_errors set to ON, you would be getting an error at the mysql_fetch_assoc() statement on the second pass through the loop that would alert you to this problem. if the challenges credits value can be different for each challenge, you won't be able to do the update using the IN() comparison as that would update all the rows using the same credits value (or write more complicated code than the task deserves.) you also would not do the update inside of the while(){} loop. the purpose of the while(){} loop is just to get an array of teams id values. any code using that array would come after the end of the while(){} loop. this code has some questionable names for table columns. you are getting the challenges table id column and using that as the teams id value. that implies the challenges id column is really the 'destination' (team2/teamB) team id. it is not the id of the row in the challenges table, which is what it should be. the `a` for a column name needs to be something more descriptive. i suspect it is the team id who made the challenge (the 'source', team1, teamA?) lastly, your code has too much code and syntax for what it is trying to do, i.e. you cannot see the forest for the trees (you cannot tell what the program logic is, because of all the clutter in the code.) here is a simplified example of what i think you are trying to do - // find any expired challenges $query = "SELECT * FROM challenges WHERE `a` = {$team['id']} AND accepted = 0 AND completed = 0 AND chtype = 1 AND expires < UNIX_TIMESTAMP()"; // the above query statement, in addition to being formed in a php variable, has been simplified by removing unnecessary php and mysql syntax. $result = mysql_query($query); // if any expired challenges are found, add back the credits to the team it is against and delete the challenge if(mysql_num_rows($result) > 0){ // at least one result found $arrayin = array(); // holds the challenges id values found while($row = mysql_fetch_assoc($result)){ mysql_query("UPDATE teams SET balance = balance + {$row['credits']} WHERE id = {$row['b']}"); // update based on the challenges 'destination'/b/team2/teamB column $arrayin[] = $row['id']; // save the challenges id values } // delete all the challenges that were just found mysql_query("DELETE FROM challenges WHERE id IN(".implode(',',$arrayin).")"); // you don't need use mysql_real_escape_string on the $arrayin values for two reasons - // 1) they are not strings and using a string function on them won't provide any protection, and // 2) they are (should be) internally generated integers and don't need any special handling. } this code assumes some things - 1) the challenges id column is the challenge row id. it is not the teams id value2) the challenges table has a column `b`, but more properly named, that does hold the teams id value that the challenge is against.3) the challenges credits value can be different for each challenge. Quote Link to comment Share on other sites More sharing options...
1st_Edition_Charizard Posted December 16, 2014 Author Share Posted December 16, 2014 Mac_gyver, your a genius. All i had to change was {$row['b']} to {$row['a']} . Column `a` is TeamA and column `b` is TeamB, but if a challenge goes unaccepted, column `a` will always be the team who posted the challenge, so just made it to a and whoola it worked, all challenges were deleted and all the credits went back to the correct team!! Your the first to totally understand the database and know what's going on, give you props. Thank you 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.