Jump to content

mac_gyver

Staff Alumni
  • Posts

    5,450
  • Joined

  • Days Won

    175

Everything posted by mac_gyver

  1. actually, yes. there are a number of programming practices and missing logic, that your code needs. 1) $showclosedgames - don't write variable names strung together like this. use underscores to separate the words that make up a variable name - $show_closed_games 2) if there are only two possible values for a variable, don't use an elseif() test, just use an else() or you can use a Ternary Operator. 3) don't use variables that end in 1, 2, ... For the posted code, you are writing out more variables than you need and then having more lines of code copying the extra variables to other variables. you should finish with the operations within one block of code before going on to the next block of code. you can then reuse common variable names like $query. 4) don't copy one variable to another. just use the original variable name in the code. 5) don't repeat yourself (DRY.) the parts of the sql query statement that are static should not be repeated. only the WHERE clause is dynamic/conditional and it is the only thing that should be part of the conditional logic. you should build the WHERE clause in a php variable, then use that variable in all the sql query statements. there will only be two sql query statements - 1) to get a count of the matching rows, and 2) to retrieve the actual data. 6) the sql query to get a count of the matching rows should use SELECT COUNT(*) so that it doesn't retrieve all the data from the table. you would fetch the count value into a php variable. 7) the purpose of getting the count of the matching rows is so that you can limit the maximum requested page number and you limit or determine if there is a next link in the pagination. the current code is not doing either of these things. if the requested page is 1, there is no previous link for the pagination. the current code is unconditionally setting up a value for a previous link. 9) the whole block of logic for the page value and sql query limiting needs help. you should get the requested page number (validating or casting it as an integer) or use page 1 as the default, then limit the page number between 1 and the maximum number of pages, then just use the final limited requested page number in the rest of the code. 10) the sql query to fetch the actual data has some error checking logic around it (which the end of wasn't included in the post, so we don't know what your code does do when there is a query error), but the first sql query to get a count of the matching rows doesn't have any error checking logic. consistency counts in programming. you should always have error checking/handling logic in your code. the easiest and most consistent way of handling sql statement errors is to use exceptions. 11) i have doubts about your $showopengames ($show_open_games), WHERE winner='' sql, and the $status open/closed logic. when $showopengames == 1, your queries are matching all the rows in the games table. when $showopengames == 0 (presumably to show only closed games), your queries are matching rows in the games table that have an empty winner value (which seems to mean games that are open.) you are then treating an empty winner value as being open and a non-empty winner value as being closed. this makes no sense. are there actually three possibilities for $showopengames - all, open, and closed, in which case the variable is not named very well, and program and query logic isn't doing what you expect? most of these points will actually simplify and reduce the amount of php and query logic, making it easier to write, test, and correct problems in your code and queries.
  2. before you can write code to perform a task, you must define what you want the result to be. if you are displaying a list of questions and three possible answers for each question, wouldn't your code need to have html markup to do each of these things? for each question, display the question text, then display the three radio buttons, each with a value that indicates which answer they correspond to, and display the answer text for each radio button. for radio buttons to work, all the buttons in a group must have the same name. i recommend using an array name, with the question id as the array index. this will let you simply loop over the submitted data using a php foreach(){} loop to get the question id and the chosen answer value. to accomplish the stated output, you would need the question text, the question id (to produce the radio button name array index and to facilitate the efficient storage of the submitted answers), and the text for three possible answers (you should actually have answer ids as the radio button values, but for simplicity, you could use the answer text as the radio button values.) does your current design have these values? what is the q1 column? is that the question text or the question id? the name of your columns should indicate the purpose of the data in the column. if you have columns named q1, q2, q3, ... this is not a good design and should be fixed. see the next point - your database design should actually store the questions (with columns for the question id and question text) and possible answers (with columns for the answer id, answer text, and if the answers are unique to each question, a question id) in separate tables (the submitted responses are stored in a third table.) if you use exceptions to handle database statement errors, you can eliminate all the conditional logic you are adding for each statement (connection and query) that can fail. an exception will be thrown for any error, which you can let php catch, and your main code only has to deal with the error free execution of statements. i also recommend that you fetch and use data from any sql query using an associative array, instead of a numerical array. this will help prevent mistakes in your code and make your code easier to read.
  3. it's not possible to write code or convert old code you have found on the web, without using the php.net documentation. next, the php mysqli extension is not the best choice to use. learn and use the php PDO extension instead. you should be using prepared queries to supply data to any sql query statement to protect against sql injection. don't use @ error suppressors, ever. all they do is hide problems and make it harder to write code that works. don't write and use functions like the fetchinfo() function. it is currently not secure and by having a function that selects a single column at a time, you will end up killing your database server with too many queries. don't write and use functions like the secureoutput() function. this function is repeating what it is doing, using a msyqli function, which has nothing to do with output, and is just nonsense code. to secure data being output to a web page, just use htmlentities() with the appropriate flag values.
  4. doing this would also help in finding the problem -
  5. to filter the status by ALL values would mean to either leave the entire status term out of the sql query statement OR cause the status term to always be a true value. the code you have posted isn't even producing the correct status terms when a value is selected. fix that problem first, and it should be clear how to make the ALL filter work. echo $sql to see what it actually is for each possibility. next, the php msyql_ extension is obsolete and has been removed from php for about one year. you need to use the php PDO extension, and use a prepared query to supply the data values to the sql query statement.
  6. did verifymail.php ever have a sess_start(); call in it? all your files must be doing the same thing for the session to match up. it's likely that this was initially working because you already had an existing session, using session_start(), that matched up between the files. when you added sess_start() to just the index.php page, that created a new session with a second name, alongside the existing session, and so your verifymail.php had session data. once you finally closed your browser and started over, index,php was using the sess_start() settings, verifymail.php was using the session_start() settings, and there was no matching session data for verifymail.php to use. most of the code you have shown at the top of index.php is common logic that ALL your pages should use. why don't you have them in a file and require it into each page (Don't Repeat Yourself - DRY) or better yet, if you are at the point of wanting to set up custom session settings, why aren't you using a single file, index.php, to implement your entire site? having a single index.php file site would eliminate the need to even pass data in session variables and would eliminate all the repetitive code and html you are trying to write, test, and maintain.
  7. presumably, before this assignment, you had assignments leading up to having a single form and the form processing code for it, hopefully on a single page, so that you weren't duplicating code and you could display any validation errors when you re-displayed the same form? this assignment, rather than just repeating that logic three times in separate files and stepping between the files, should be more about implementing this all on a single page, which would eliminate all the triplicated logic and html markup. when you move from one form to the next, you would just submit a hidden 'step' value with the form data. the code on a single page would use the step value to control what the page does. you can produce previous/next step form buttons (like pagination) to let the visitor move between steps. if not on the first step, you would display a previous button. if not on the last step, you would display a next button. if on the last step, the next button would become the final submit button. the form processing code would use previous/next input to modify the step value if the submitted form data is valid (you would stay on the same step number if the data isn't valid.) the form processing code would also use the submitted step value to control what it does. the submitted step value would tell the form processing code which set of inputs it should expect. the code would validate the input data and if the step is not the final form submission, store the validated data in a session variable (i would use the step number as a sub-array key for store the data from each step.) forget about trying to use an OOP class for any part of this, get the program logic to work first. if the submitted data is from the final form submission, after you validate the inputs, use all the submitted data for whatever purpose you need.
  8. if you simplify the cart so that it uses the item_id as the array index and stores the quantity as the array value, all the code will be greatly simplified. when you add something to the cart, you would need to submit the item_id and the quantity. both these values should be submitted as post data. after you validate both submitted values, this is all the logic you need to either initially insert the item into the cart with the submitted quantity or to add an additional quantity to an existing item - $item_id = (int)$_POST['item_id']; // if the item is not in the cart, add it with quantity 0 (the next step will add to the quantity) if(!isset($_SESSION['cart'][$item_id])) { $_SESSION['cart'][$item_id] = 0; // add the item id to the cart with quantity 0 } // add the submitted quantity to whatever is in the cart for this item $_SESSION['cart'][$item_id] += (int)$_POST['quantity'];
  9. the suggestion bsmither made has to do with a variable you are assigning a value to on line 3 in the posted code and a later line of code that's supposed to be using that same variable to supply a value to the sql query statement. they don't match and you would have probably gotten them correct if you had just written the code yourself rather than copy it from somewhere on the web. here's a laundry list of things your code should/shouldn't be doing - 1) use a get method form for a search function. 2) don't use $_REQUEST variables. Use the proper $_GET, $_POST, or $_COOKIE that you expect the data to be in. 3) when you validate input data, set up and output a message to the visitor when that data isn't valid. what happens now if a submitted name doesn't match your preg_match() statement? don't leave the visitor wondering why a web page doesn't display anything. 4) use a prepared query to supply data to the sql query statement. your current code, that's putting the (wrong) php variable directly into the sql query statement, is open to sql injection (someone could inject sql and get your current code to display the complete contents of any database table you have.) unfortunately, the php mysqli extension is not the best choice to use, ever, and more so with prepared queries. if you can switch to use the php PDO extension. 5) don't unconditionally output database errors to the visitor (your connection code now). unfortunately, your code isn't even checking for an error when it runs the sql query statement. the easiest way of checking for errors is to use exceptions. this will eliminate the need to add program logic at each database statement that can fail with an error (the connection and query statements.) if you let php catch the exception, it will use the php error_reporting/display_errors/log_errors settings to control what will happen with the actual error information. when learning, developing, and debugging php code, you would display all errors. on a live server, you would log all errors. 6) for a search, if the query doesn't match any data, set up and output a message to the visitor telling them so. again, don't leave the visitor wondering why a web page isn't displaying anything. 7) when updating old mysql_ based code, you have to update all the mysql_ statements. you missed one and there would be a run-time php error alerting you to the problem. do you have php's error_reporting set to E_ALL and display_errors set to ON (preferably in the php.ini on your development system) so that php would help you by reporting and displaying all the errors it detects? you should separate the database specific code (that knows how to query and retrieve data) from the presentation code (that knows how to produce the output from the data.) doing this will make it easier to test your code and make your code more general purpose and easier to reuse. the way to do this is to store the fetched data into a php variable in the database specific code, then just use that php variable in the presentation code. this also makes it easy to implement item #6. you can just test if the php variable is empty or not to output the message to the visitor. 9) there's no good reason to copy values from one variable to another. when you switch to use a prepared query for the search, you would just use $_GET['name'] in the code. this would eliminate (one of) the current problems in the code. when you loop over the data from the query, don't copy values from the $row[...] variables into other variables. this is just wasting your time typing (and i'm pretty sure you are not doing this as part of a typing class.) just use the $row[...] variable where it is needed. 10) when outputting dynamic/variable values on a web page, because they can consist of any value that was allowed to be input/created, you need to apply htmlentities() to the output. 11) you can put php variable directly inside of a double-quoted string. this will simplify your code when producing output. you have a mess now and there's at least one extra </a> and some missing or misplaced <li></li> tags. 12) you should validate the html that your code outputs at validator.w3.org this would help find some of the problems mentioned in item #11. 13) if you store your database connection credentials in an external .php file and require it into your main code, you won't have to take the time to edit code when posting it on a help forum.
  10. when you update old mysql_ based code, you must convert all the statements and when debugging php problems, get php to help you by setting error_reporting to E_ALL and display_errors to ON. you missed a mysql_real_escape_string (and a few mysql_error()) statements that would be throwing php errors to help pin down the problem. the reason this works on one system and not another is because php and the all in one system creators thought it would be funny to set up default database connection credentials and to have mysql_ statements try to make a connection when there isn't one. on the system where this doesn't work, there are probably no default connection credentials, so the mysql_real_escape_string function call can't make a connection and it returns a null/false value instead of the escaped search term.
  11. if you are updating ALL the 'data' columns that exist in a row for any index value, you might as well just replace the row. i don't know how the performance of this multi-value method compares with the multi-value INSERT ... ON DUPLICATE KEY UPDATE ... method.
  12. just because mysqlnd is installed, doesn't mean that the mysqli or PDO extension will use it. your installation of php would need to be compiled with switches set causing the mysqli and/or PDO extensions to use the mysqlnd driver. there would be mysqlnd entries shown in the client api sections of the msyqli/pdo_mysql phpinfo output. if the script cannot be easily switched to use the PDO extension, you may (untested) be able to conditionally detect the existence of the get_result() method and extend the mysqli stmt class with a user written method that returns an instance of a user written class that emulates the features of a mysqli result class that the script is using (afaik, it is not possible to create and return an actual populated instance of the built-in mysqli result class). this of course is just a kluge. the whole php mysqli prepared query implementation is bad, and this is just one more case highlighting that it should not be used.
  13. the HEREDOC ending tags ( END_OF_TEXT; in your code) must start in the 1st column and be the only thing on the line. it's not clear if what you posted was the result of how you added the line numbers or if you actually have some white-space ahead of the Heredoc ending tags. you also have at least one weird single-quote, in front of the l_name array index, on about line 122, that needs to be a simple single-quoted - there's a missing single-quote on about line 140, ahead of the state array index name - there's another weird quote on about line 175, ahead of the add_type array index name - and there's more after that point, but i stopped looking. you can find these type of things by looking at the color highlighting, or lack of, in your programming editor. at each of these, the color highlighting stopped changing at that point. edit: here's some more suggestions - 1) use exceptions to handle database statement errors. this will eliminate all the logic from the code that's testing if the queries (and connection) worked. 2) use prepared queries. this will eliminate all the mysqli_real_escapes_string function calls from the code and all the extra variables being used to hold the escaped data. 3) if you are building a double-quoted php string, rather than escaping double-quotes within the string, just use single-quotes within the string. 4) you can put php variables directly inside a double-quoted php string. no need for a bunch of concatenation dots. these things will greatly simplify your code, so that you/we/i can see what it is trying to do.
  14. if c1, c2, and pk in your example are the only columns, you can use a multi-value REPLACE query, otherwise use a multi-value INSERT ... ON DUPLICATE KEY UPDATE .. query (which, since the the data already exists, implements a multi-value UPDATE query). the max packet size refers to the data sent in each communication back and forth between php and the database server. for a true prepared query, the sql statement (which is a string) would be sent in one communication, and the data for each execute statement would be another communication. named place-holders are implemented solely in the php PDO driver. the actual sql statement that is sent to the db server has had them replaced with ? and the driver maps the names to the place-holder parameter number when it builds the communication command to send the data to the db server - here's an interesting read on what the binary transfer protocol is for the execute statement when using prepared queries - https://dev.mysql.com/doc/internals/en/com-stmt-execute.html#packet-COM_STMT_EXECUTE note: the binary transfer protocol is only available when using prepared queries and then it only really saves time if transferring binary data. for string data values, which is the default for PDO, unless you specifically bind the data using a non-string data type, the data is still sent as characters and there's no time savings in the communication. and here's the reason for the performance difference - for simple queries, the communication time (handshaking + actual transfer) for both the prepare() and execute() statements is much greater than the time to actually prepare (parse/plan) or to execute the query on the database server, so, from the php side, the only way to significantly reduce the amount of time taken is to reduce the number of separate communications. running a single row prepared query inside of a loop only saves about 5% (bench-marked using a mysqli prepared query a while ago) of the overall time, compared to running a non-prepared query inside of a loop, because all you have eliminated from the loop code is the communication of the sql statement string and the parse/plan step. you are still performing a separate communication for each pass through the loop and have to spend the time for the handshaking + actual transfer for each communication. the only signification time savings i have seen is when using a multi-value query, which Barand has also confirmed in his tests. to fully implement a bulk method, figure out the maximum number of data values you want to send at once. if the actual amount of data is less then the maximum you have chosen, dynamically produce the multi-value sql statement of your choice for that amount of data, prepare it, then supply the data as an array to the execute statement. if the actual amount of data is greater than the maximum you have chosen, break the data into equal size blocks that are less than the maximum. if there's an odd amount of data, you can include one dummy/repeat set of values to make all the blocks the same size. then, dynamically build the multi-value query statement with the correct number of sets of place-holders to match the block size, prepare the query, then loop over the actual data and break it into arrays of the correct size, fixing up the size of the last block if it is odd, and supply each array of data to the execute() statement.
  15. another point about prepared queries, you prepare them once, then can execute them multiple times. the UPDATE query should be prepared once, before the start of the loop. the code inside the loop should only populate the data for the place-holders, then execute the query. @NotionCommotion, the OP's from and to values do make sense. from is an older date and needs to be the first parameter in the BETWEEN term for the statement to work. to is a newer date and needs to be the second parameter in the BETWEEN term.
  16. do you have php's error_reporting set to E_ALL and display_errors set to ON in the php.ini (putting these settings into your code won't help show syntax errors in the same file where the settings are being set) on your development system so that php would help you by reporting and displaying all the errors it detects? you would be getting a php syntax error because you are mixing php and html markup inside of php code tags. you would also be getting a blank php page, which you didn't tell us you are getting as a symptom when you run your code. to output in-line html markup, you need to have a closing php tag to switch out of php 'mode'. with 300+ posts, you should be past having these basic problems.
  17. example of the array/flag method, that has the added advantage of separating the database specific logic from the presentation logic, so that if you do end up with a correctly designed database table, you only have to change the database specific logic. the presentation logic would remain the same. if (isset($_GET['zoeknummer'])) { $pdo = new PDO("mysql:host=$dbhost; dbname=$database", $dbuser, $dbpass); $pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); $pdo->setAttribute(PDO::ATTR_DEFAULT_FETCH_MODE, PDO::FETCH_ASSOC); $pdo->setAttribute(PDO::ATTR_EMULATE_PREPARES, false); $sql = "SELECT lead_content FROM wp_wgbsupicrm_leads"; $res = $pdo->query($sql); // where $pdo is your db connection $matches = array(); // array/flag to hold the matching data while($lead = $res->fetchColumn()) { $data = json_decode($lead); if ($data->zoeknummer == $_GET['zoeknummer']) { $matches[] = $data; } } // presentation logic - produce the output from the data if($matches) { foreach($matches as $data) { echo "Uw plaats voor " ; echo $data->wachtlijstkomplex . ' : ' . $data->wachtlijstplaats . '<br>'; } } else { echo 'Geen resultaat gevonden met opgegeven nummer. <br> Kijk na of U het nummer correct heeft ingevuld.'; } }
  18. if you are not tied to this database design, you should change it to properly store the data. by storing json encoded data, you cannot directly find matching data using the sql query. this will also not scale well as you must retrieve all the data in order to scan it for matches. this will result in a poorly performing web site when there is a moderate to large amount of data. if the data is stored properly, by storing the 'columns' in the json data in individual database table columns, you can find the row(s) that match the search term, directly in the sql query statement and all the code will be simplified. for your current design, the easiest way of using the FLAG method would be to use an array and store any data that matches the search term into the array. after you finish looping over the data, if the array is empty, there were no matches. if the array is not empty, loop over the entries in it and produce the output from the data.
  19. please, don't try to store data into a database table like it is a spreadsheet. each data item should be stored in a separate row in the table. your table should have columns for KODARTIKULLI, KODNIVELCMIMI, and CMIMI. next, you don't have to try to select data (which you are not actually fetching and storing the count into a php variable in your code) to determine if you are going to insert a new row or update an existing row. there's a single query that does that - INSERT ... ON DUPLICATE KEY UPDATE ... the KODARTIKULLI and KODNIVELCMIMI columns would be defined as a composite unique index to both enforce one row per combination of those values and to allow this query to work. you should be using a prepared query in any case, but using one when running a query inside of a loop will result in the most efficient operation (saves some time in the parsing and planning of the sql statement). the query would be prepared once, before the start of your loop, with place-holders in the sql statement for the data, then the data would be supplied to the sql query statement when you execute the query inside of the loop. unfortunately, the php mysqli extension is not the best choice to do this. if you can you should switch to use the php PDO extension. in short, all the code and queries you have shown can be replaced with just a few lines of code and one sql query statement.
  20. putting the error settings in your file won't help with php syntax errors in the same file because the code never runs to cause the settings to take effect. you need to put these settings into the php.ini on your development system, which may require restarting your web server to get the changes to take effect. BTW - the currency symbol should not be stored with the price (hint as to where at least one error is at). it is a display property and should be handled when you display the price, not when you store the price.
  21. vague comments about not having any luck with something don't tell us anything useful. there are varying levels of luck and unless we know what your standard is, we don't know what result you are getting. communicate exactly what is happening and if it's not blatantly obvious what's wrong with the result, tell us what's wrong with the result and what result you expected. do you have php's error_reporting set to E_ALL and display_errors set to ON (preferably in the php.ini on your development system) so that php would help you by reporting and displaying all the errors it detects? by default, you cannot use a URL with file_get_contents(), and there would be a php error alerting you to this issue.
  22. how do you know the email is not being sent? what exact symptom, error, or output are you getting from your code and what output did you expect? where in the posted code are you echoing the message at? how do you know that the $email isn't somehow satisfying the mail() function. there are many levels of satisfaction and unless we know your standard, we are left guessing what that statement means. short-answer: were are not there with you and don't know what you saw when you ran your code. the information you supply must concisely communicate what did happened, what should have happened, and for the case of echoing things in the code, post that code, not some other code.
  23. since you didn't address each of the points/questions i ask, it's not going to be possible to directly help you, since we only see the information that you post. these are some more points, from your last thread - i'm betting your posted code is either not being executed due to conditional statement(s) around it being false or it is being executed and is producing output, but you are not seeing it due to this combination of coding and php's stupid output buffering setting. if you want exact and direct help with what your code is doing, you will need to post all of it, so that we aren't guessing about what it may be doing.
  24. are your form fields within a valid post method <form></form>? is your html valid? if it's not, the form fields could be broken and not be considered by the browser to be form fields. what exact post data is being submitted? do you have php's error_reporting set to E_ALL and display_errors set to ON (in the php.ini on your development system) so that php would help you by reporting and displaying all the errors it detects? is the posted code the complete file? it's missing at least two closing } that would producing a php syntax error. lastly, in addition to the questionable statements and logic being used, to provide an audit-trail for the transactions, which also helps in debugging program operation, you should not just add/subtract amounts in a database table field. you should store each plus or minus transaction as a row in a table. to get the current total, you would just SUM() up the values for any user.
  25. web servers and browsers are stateless. they don't know anything about any http request before or after the current one. when you refresh the page, it is requested again and it starts over and operates on any current data it is told to use. the $scope.comments array, that's hard-coded now, should instead be retrieved or dynamically built from the stored data on the server. by appending/pushing the submitted data to the $scope.comments array in the client, you have duplicated data on the client and on the server and can have the data out of synchronization if the server side code doesn't validate and store the data. data should only be stored in one place. next, this line - $json = file_put_contents('names.json', file_get_contents('php://input'), FILE_APPEND); isn't doing what you think. file_put_contents() returns an integer that's the number of bytes written to the file (or a false value if the file_put_contents() failed), so, while this will append the submitted data to names.json, the $json variable isn't either the submitted data or the entire contents of names.json. the reason you are getting php errors from the code is because $json, and then $data, isn't the submitted data. btw - don't use @ error suppressors in your code. they just hide problems, while still leaving your code nonfunctional. your task as a programmer is to find and fix problems, not hide them. this would be a good point to mention separation of concerns. i don't know from your program logic what you expect $json to hold, but if you separate the different concerns in your code, it would be clear to you and us what you are trying to do. saving data due to a form submission/post request is a separate concern from retrieving data to output it. your server side post method form processing code should detect that a post method form was submitted, input and validate the submitted data, then if there are no validation errors, use the submitted data. for what your application is doing, using the submitted data would mean to store it persistently on the server. you can display/dump the data for debugging purposes, but that is not the primary concern of the post method form processing code. if you then want to retrieve some or all of the the stored data and output it, this is a separate concern from the post method form processing and the code to do this should be a separate independent section in your file.
×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.