Psycho
Moderators-
Posts
12,157 -
Joined
-
Last visited
-
Days Won
129
Everything posted by Psycho
-
That's not how you should do that. That query is returning ALL the records from the table and then you are getting the count of those records. That is very inefficient. There is no need to have the query return ALL the data, just to get the count. Use the query you had previously so the result set only returns the count. Then get the value form the result set. I don't use that type of syntax, so I'm not 100% sure, but it should go something like this $q = DB::getInstance()->query("SELECT COUNT(*) FROM records"); $row = $q->fetch_assoc(); $total = $row[0];
- 9 replies
-
- php
- pagination
-
(and 2 more)
Tagged with:
-
You're right. I missed that. I almost never see anyone use that format. I thought that was why he was getting "all" the records.
- 9 replies
-
- php
- pagination
-
(and 2 more)
Tagged with:
-
You should really break apart the code into functions based upon specific activities to be accomplished. For example, a function to run the query and return the results would be helpful. In any event, I'm pretty sure this is at least part of the problem: $total = DB::getInstance()->query("SELECT COUNT(*) FROM records"); $total is the result resource of the query not the value returned from the query. You need to extract the value. Also: $stmt = DB::getInstance()->query("SELECT images.*, records.* FROM records LEFT JOIN images ON records.user_id = images.user_id ORDER BY record_id LIMIT {$limit} OFFSET {$offset}"); The parameters for limit are [start_position] and [count]. You have them reversed.
- 9 replies
-
- php
- pagination
-
(and 2 more)
Tagged with:
-
The data that can be returned from the different game server and the optional parameters that can be used when making those queries will vary greatly. But, instead of trying to build the low level code to make those connections and retrieve data, you might want to see if there is already something existing that supports all the game servers you want such as this: http://gameq.sourceforge.net/ (>170 games supported). Although, I see that there are several more current bames not supported (BF3 & BF4). I would start with a package like that. Then for the other games you want to support, see if there are any similar existing packages. Then you can build a middle-tier class that will communicate with them all. E.g. based on the game data being requested, your class would determine which package to use and return the data in a consistent format. Then you would have just one interface for getting data.
-
No. How an API works is totally up to the programmer that creates it (or the requirements they are given). They can make whatever input parameters they think are needed and can have whatever output formats that they want. That's why they (should) have documentation.
-
Well, yes and no. The documentation for the API is located here: http://bf4stats.com/api Read through that to determine what parameters to pass to only get the data you want. But, it's likely you will still get more data than you are after. So, just parse the return data to use the data you want to display. The url you provided has an 'output' parameter. Look at the documentation and the available output formats to determine which one you want to work with. EDIT: I don't see anything in the returned data that includes what game the user is playing. <?php //Set player name for the query $playerName = '1ApRiL'; //Get the data in JSON format $raw_data = file_get_contents("http://api.bf4stats.com/api/playerInfo?plat=pc&name={$playerName}&output=json&opt=details"); //Decode the data into an object $data = json_decode($raw_data); //Get player object $playerObj = $data->player; echo "ID: {$playerObj->id}<br>\n"; echo "Name: {$playerObj->name}<br>\n"; ?> Note: the opt=details added to the URL will return only the 'basic' details - which include the name and id. The documentation includes more info on the other parameters that can be used.
-
I made a new template for site. Is site better?
Psycho replied to holly9's topic in Website Critique
I don't know what type of input you are looking for. But, here are some things I see: The text in the header "Free script and codes" can appear on top of the logo if users makes page narrow. You can implement a minimum width div or put a large left margin on that text to ensure that does not happen. Also, the text should have aright-hand margin. The floating content is unnecessary as you currently have it. Resizing the page does not allow the user to increase/decrease the available horizontal space for the text to display. I would go with a left-hand fixed position for the content (or allow it to resize with the page). The home page has no links to content. The bulleted list looks like it should be links to something Spelling and grammar issues throughout. The Games page starts with No results found until the user does a search. Should have something by default. Validation on contact page needs improvement. Allowed me to submit an 'empty' code and content consisting of nothing but spaces. The TOS are, to be blunt, a joke. Go do some research on appropriate TOS. Really? That's sort of like my ISP telling me they will send me an email to tell me that the email service is down. I used to laugh at the fact that the electric company would post notices and information about power outages when they started doing that many years ago. But, since the prevalence of smart phones that makes sense these days. Site Map: You use the same display of the links on the site map as you do for the bullets on the home page. You should not use the exact same display property for content that will be links and content that will not be links. The styles you use should help guide the user to what things are and should be consistent through out the site both in display and purpose. Also, it is not apparent that the 'headers' on this page are links as well. A quick look at the source on a couple pages shows some invalid HTML. You should run your pages through a validator. -
As ginerjm stated, this is likely your problem while($row = mysql_fetch_array($query1)) { (list($id, $name) = mysql_fetch_array($query1)); The while() loop condition gets the next record from the result set ($query1 - odd name for a result). Then the first line in the code vlock for the loop attempts to get another record from the result set (without doing anything with the results from the record retrieved in the while() condition). The result is that only every other record would actually be processed: 1, 3, 5, 7, etc. Also, use fetch_array() with list is a poor idea in my opinion. The reason is that fetch_array() returns both numerically indexed and named indexes of the results. You don't show the query used, but if you ever changes the SELECT part of the query or if you change the DB structure (if you are using *), the code will break. There is no need to create variables of $id and $name since you will likely only use those values in the loop once or twice. Just use the $row array value created in the while() loop and get rid of that other line. Some other comments: - Don't use urlencode() for content that is not part of a url. If it is displayed on the page it should use htmlentities or htmlspecialchars - Add a \n at the end of echo'd lines in the output. It will put a linebreak in the output and make it much, much easier to read the HTML output and find/debug errors - Try not to go in and out of PHP/HTML too much. You have some invalid output probably because you couldn't "see" it due to that. (one of the hyperlinks has two closing tags). plus, you didn't use urlencode on the second hyperlink for the 'id' value on the query string - Separate the logic (PHP) from the output/content (HTML). When processing the logic, put the output into variables and then just echo the variables in the HTML. Ideally you should have separate files for the processing code and the output code. But, to start, just put the processing code at the top of the page, then put the output at the bottom. - There is output before you have code that will send headers(). You can't send headers if you have ever sent output for the browser (such as an echo). The code to output the list of records comes before the code to send the file - which is why you need the objflush() is why you are having to use ob_clean() and flush(). Why put the code to create teh page output before the code to send the file? That logic should be reversed. No need to run code to generate output if there is a chance it won't be used. - The query to get the content is using the GET value directly in the query (open to SQL injection) - There is no handling of the code to get the file if there are no results from the query Here is something closer to what I would do <?php if(isset($_GET['id'])) { // if id is set then get the file with the id from database //$con = mysql_connect('localhost', 'root', '') or die(mysql_error()); //$db = mysql_select_db('test', $con); $id = intval($_GET['id']); $query = "SELECT name, type, size, content FROM requisition WHERE id = '$id' LIMIT 1"; $result = mysql_query($query) or die('Error, query failed'); if(!mysql_num_rows($result)) { echo "File not found"; } else { list($name, $type, $size, $content) = mysql_fetch_array($result); header("Content-length: {$row['size']}"); header("Content-type: {$row['type']}"); header("Content-Disposition: attachment; filename={$row['name']}"); echo $content; mysql_close(); } exit; } //Put code here to run the query to get the list of data $tableOutput = ''; while($row = mysql_fetch_assoc($query1)) { $idURL = urlencode($row['id']); $nameHTML = htmlentities($row['name']); $statusHTML = htmlentities($row['status']); $tableOutput .= "<tr>\n"; $tableOutput .= "<td> $row[category]</td>\n"; $tableOutput .= "<td> $row[description]</td>\n"; $tableOutput .= "<td> $row[amount]</td>\n"; $tableOutput .= "<td><a href='approved_req.php?id={$idURL}'>{$nameHTML)}</a></td>\n"; $tableOutput .= "<td> {$statusHTML}</td>\n"; $tableOutput .= "<td><a class='btn btn-success' href='invoice.php?id={$idURL}'> Pay </a></td>\n"; $tableOutput .= "</tr>\n"; } ?> <html> <head></head> <body> <table> <?php echo $tableOutput; ?> </table> </body>
-
Yeah, I see that could cause confusion. That first paragraph was in reference to using a for() loop with an incrementing variable to loop through the array values is "wrong" in almost all cases (there are a few cases where it makes sense). It assumes the array is numerically indexed with sequential numbers. A foreach() loop guarantees you will iterate through all the values in the array.
-
(Beginner) Defining a string in a separate file...
Psycho replied to jyeager11's topic in PHP Coding Help
Why modify the original code that uses a less optimal process? THe code I provided does just what you ask. At least it did in the limited testing I did. -
No
-
According to the FPM site
-
Why did you change this to use urlencode() instead of mysql_real_escape_string()? Those values are from a url so, if anything, you would use urldecode(). BUt, even that doesn't protect against SQL injection - which is why I had used mysql_real_escape_string(). No. A modulus will not return .67. It will return a whole number. If you use 3 as the number of rows, modulus will return the remainder of record_count divided by three, which will have only three possible values: 0, 1 or 2. As for the problem with the output - I had one copy/paste error on the use of the modulus which cause the problem. The condition to close a row should be this (Note the 1 is now a 0): //Close row if last record in a row if($row_count%$max_columns == 0) { $output .= "</tr>\n"; } I created some mock data to test and all seemed correct. In fact try the script with different values for $max_columns and it should "just work". That's how well written code should work. You should not have to completely rewrite a page because you want to change the output for something trivial as the number of columns.
-
Hmm, . . .after further consideration I'm not sure I understand your intended output. The code above would put each record in a cell going left to right, top to bottom like this ------------- | 1 | 2 | 3 | ------------- | 4 | 5 | 6 | ------------- | 7 | 8 | 9 | ------------- But, looking at your example page, I think you may just want three cells with the records grouped into the cells like this ------------- | 1 | 4 | 7 | | | | | | 2 | 5 | 8 | | | | | | 3 | 6 | 9 | ------------- Or maybe some other variation of those two. If the format I provided isn't what you want, please give some details. Making that change will be relatively easy with what I already did.
-
OK, I had some difficulty following some of the logic to determine what the output should be and I'm seeing some problems in the code you have. For example, there is a closing anchor tag: (</a>) after the image, but no opening tag. Anyway, this process allows you to have as many columns as you want by just defining $max_columns at the top of the script. This is only one set of code for outputting records (not multiple for each column). The magic that makes this happen is the modulus operator: '%'. A modulus gives you the remainder of a division. So, if you divide the row count by the max columns (3) you will see a pattern: Row 1 : Modulus of 3: 1 Row 2 : Modulus of 3: 2 Row 3 : Modulus of 3: 0 Row 4 : Modulus of 3: 1 Row 5 : Modulus of 3: 2 Row 6 : Modulus of 3: 0 So, when the modulus is 1 you start a new row. When it is 0 you close the row. I also created a function for creating the output for a single record. This helps to compartmentalize the logic and separate the content from the logic. I don't have your database, so I did not test this. There may be some syntax errors or other minor errors - but the logic should be sound. I don't know why you are using strip_slashes() for the output. You should be using htmlentities() or htmlspecialchars(). I suspect you are storing 'bad' data which is forcing you to remove the slashes. You should look at sanitizing the data before it is stored. Then only use the proper method for escaping based on the output. A value used in a url should use urlencode() and output as 'HTML' should use one of the functions mentioned above. You're also using the deprecated mysql_ functions (which I didn't change). <?php //Define the number of columsn to use $max_columns = 3; //Function to create output for one company function companyOutput($row) { //Create output for the company $company = ''; if(!empty($row["m_logo"])) { $imgURL = stripslashes(urlencode($row["m_logo"])); $company .= "<div align='center'>\n"; $company .= "<img src='http://marketingteammates.com/_ZABP_merchants/_zcnsLogos/gick.php/{$imgURL}?resize(220)' border='0'></a><br />"; $company .= "</div>\n"; } $company .= "<h6>" . stripslashes($row["mS_MGMT_m_company"]) . "</h6>\n"; $company .= stripslashes($row["b2c_m_address1"]) . ' ' . stripslashes($row["b2c_m_address2"]) . "<br />\n"; $company .= stripslashes($row["b2c_m_city"]) . ' ' . stripslashes($row["b2c_m_state"]) . ', ' . stripslashes($row["b2c_m_zip"]) . "<br />\n"; $company .= "Phone: " . stripslashes($row["b2c_m_phone"]) . "<br />\n"; $company .= "Fax: " . stripslashes($row["b2c_m_fax"]) . "<br />\n"; $company .= "<a href='mailto:" . stripslashes($row["b2c_email"]) . "'>" . stripslashes($row["b2c_email"]) . "</a><br /><br />\n"; $company .= "Hours of Operation: " . stripslashes($row["m_coupon_title"]) . "<br />\n"; $company .= stripslashes($row["m_coupon_desc"]) . "<br /><br />\n"; $company .= "<div align='center'>"; $company .= "<img src='../images/print_image.gif' width='29' height='31' /><br /><br />\n"; $company .= "<hr width='80%' size='1' />\n"; $company .= "</div><br /><br />\n"; return $company; } //Get data from the url - and make SQL safe $UN_URL = mysql_real_escape_string($_GET['un']_; $mS_MGMTcampaignID_URL = mysql_real_escape_string($_GET['mS_MGMTcampaignID']); //Create and run query ### DON'T USE * IN THE SELECT - IT IS INEFFICIENT $query = "SELECT mS_MGMT_m_company, m_logo, b2c_m_address1, b2c_m_address2, b2c_m_city, b2c_m_state, b2c_m_zip, b2c_m_phone, b2c_m_fax, b2c_email, m_coupon_title, m_coupon_desc FROM manager_Specials_company_mgmt MSCM LEFT JOIN b2c_coupons B2CC ON (MSCM.mS_MGMT_id = B2CC.m_primeID ) WHERE mS_MGMTcampaignID = '$mS_MGMTcampaignID_URL' ORDER BY mS_MGMT_m_company ASC" $result = mysql_query($query); $result_count = mysql_num_rows($result); //Create the output $output = ''; if(!$result_count) { //There were no results $output .= "<tr><th>There were no results</th></tr>\n"; } else { //Redefine max_columns if total is less than the max //i.e., if there are only 2 records, only use 2 columns $max_columns = min($max_columns, $result_count); //Create header row $companyString = ($result_count==1) ? 'company:' : 'companies:'; $output .= "<tr>\n"; $output .= "<th colspan='{$max_columns}' valign='top' class='body_text_10_Center'>"; $output .= "{$mS_campaignName_mS} is proud to sponsor {$result_count} {$companyString}"; $output .= "</th>\n"; $output .= "</tr>\n"; //Create output for each result record $row_count = 0; while($row = mysql_fetch_assoc($result)) { //Increment the row counter $row_count++; //Open new row if first record in a row if($row_count%$max_columns == 1) { $output .= "<tr>\n"; } //Output the record $output .= "<td>\n" $output .= companyOutput($row); $output .= "</td>\n" //Close row if last record in a row if($row_count%$max_columns == 1) { $output .= "</tr>\n"; } } //If last row was not closed, close it now if($row_count%$max_columns != 1) { $output .= "</tr>\n"; } } ?> <html> <head></head> <body> <table width="944" border="1" align="center" cellpadding="0" cellspacing="0"> <?php echo $output; ?> </table> </body> </html>
-
Um, yeah. Take your time and build code that is logically structured things like this are easy. You should be able to have a variable to define the number of columns and be able to change it at will and it just works. Also, don't intermingle PHP logic and the output. It makes it really hard to manage code. I'll post something in a few minutes.
-
(Beginner) Defining a string in a separate file...
Psycho replied to jyeager11's topic in PHP Coding Help
Well, the current code can be improved. There is no reason to explode the source text. Just use string functions for this. But, to get the contents of an external file you would use the function file_get_contents() I would put the process into a function to make it more extendable function replaceContents($inputString, $startTag, $endTag, $replaceString) { //Determine positions for replacement $startPos = strpos($inputString, $startTag); $stopPos = strpos($inputString, $endTag) + strlen($endTag); $length = $stopPos - $startPos; //Make replacement and return result $output = substr_replace($inputString, $replaceString, $startPos, $length) . "\n"; return $output; } $poststart = "<!-- Begin -->"; $poststop = "<!-- End -->"; $replaceString = file_get_contents("replace.txt"); $contenu = replaceContents($contenu, $poststart, $poststop, $replaceString); -
I would go further than just stating using foreach() on an array is easier to read and write. I think it is downright wrong in 99% of cases. It assumes that the indexes of the array are numerically based and sequential. While that may be the case when you know the source of the array when writing the code, you don't know if the array format may be changed in the future such that the process would fail. A foreach() loop is much more fool-proof and resilient if the intent is to iterate through the values of an array. I've also been seeing a lot of instances on the forum where people are getting a count of the results from a DB query and then using that count to run the loop to get the records - either in a while()or a foreach(). While that is slightly less problematic since the count of records will not change I would also consider it wrong. A while() loop using the ability to get the next record is the right way while($row = mysqli_fetch_assoc($con, $result)) {} I've never seen it, but an error could occur when getting a record from the result set. Using a while() loop in that manner ensures that the page doesn't fail with an unhanded exception.
-
Then I don't think your issue is with loops. In this situation, just assume you have ONE record to compare and update in the database. Create that process. Then create a loop to get one record at a time and perform the same process. If you can do it once, you can do it many times.
-
//get the csv file $file = $_FILES['csv'][tmp_name]; $handle = fopen($file,"r"); I'm not a fan of defining a variable to only use it once. Why not just . . . $handle = fopen($_FILES['csv']['tmp_name'], "r"); I would trim the values //assign the variables $firstName = trim($data[0]); $lastName = trim($data[1]); $email = trim($data[2]);
-
Several issues in that code: 1. You should not use 'external' variables to create a while() loop. Use a while() loop to get one row at a time. Makes the code much, much simpler. What you have now could easily break if the values for $i or $j get modified incorrectly. Not to mention that variables with names like $i and $j provide no context as to what they hold. 2. No reason to call mysql_result() many times instead of just calling a mysql_fetch_ one time. As to your question it can be implemented many ways. You can create ONE form and have a submit button for each record which will pass the ID of the record to your edit page. Or, you can make a mini form for each record. Which one you go with will be determined by several factors. You also need to decide what 'value' is passed to the edit page (I'm assuming it is the fkey). Here is one example: <?php require('connect.php'); session_start(); $query = "SELECT fkey, projectid, paying, completionmonth, completiondate, personspecify, projecttitle, categories, tools, shortdescription, date, time FROM project WHERE projectstatus='Open'"; $result = mysql_query($query); $count = 0; $projectList = ''; while ($row = mysql_fetch_assoc($result)) { $count++; $projectList .= "<br>{$count}<br>\n"; $projectList .= "<br>{$row['projectid']}<br>\n"; $projectList .= "<br>{$row['fkey']}<br>\n"; $projectList .= "<br>{$row['paying']}<br>\n"; $projectList .= "<br>{$row['completionmonth']} month {$row['completiondate']} days <br>\n"; $projectList .= "<br>{$row['personspecify']}<br>\n"; $projectList .= "<br>{$row['projecttitle']}<br>\n"; $projectList .= "<br>{$row['categories']}<br>\n"; $projectList .= "<br>{$row['tools']}<br>\n"; $projectList .= "<br>{$row['shortdescription']}<br>\n"; $projectList .= "<br>{$row['date']}<br>\n"; $projectList .= "<br>{$row['time']}<br><br><br><br>\n"; // I want to insert edit button here but dont know how $projectList .= "<form action='edit.php' method='post'>"; $projectList .= "<input type='hidden' name='fkey' value='{$row['fkey']}'>"; $projectList .= "<button type='submit'>Edit</button>"; ec$projectList .=ho "</form><br>\n"; } ?> <html> <head></head> <body> <?php echo $projectList; ?> </body> </html>
- 4 replies
-
- php
- formbuttons
-
(and 1 more)
Tagged with:
-
OK, that error is just what it means. You are trying to 'use' a variable that has not been defined. But, you don't state for "which" variables you are getting that error. I know you may not have a lot of experience, but providing the actual errors you are receiving would seem like an obvious thing to include. So, with the undefined error. These are likely warning/notice errors (I forget which one). These can be suppressed (and should be in a production environment). But, you want these errors on in a development environment because it helps to prevent hard to find defects. For example, If you define a variable $data and later try to reference that variable using $Data. Here are a couple places that I could see you getting that error $product_id = $_GET[id]; //the product id from the URL $action = $_GET[action]; //the action from the URL If you were to access the page and didn't include variables on the query string for 'id' and 'action' then you are referencing values that don't exist. If the errors were suppressed and $_GET[id] does not exist, then $product_id would be defined as a NULL value and, if your page was coded appropriately, everything would work just fine. But, as I stated above you want to have these errors enabled in development so you can ensure the code is of high quality. What I typically use in this situation is the ternary operator (a shorthand if/else) to test if the GET value is defined and set the variable accordingly $product_id = isset($_GET['id']) ? $_GET['id'] : false; //the product id from the URL $action = isset($_GET['action']) ? $_GET['action'] : false; //the action from the URL NOTE: Textual array indexes should be enclosed in quote as in my example above. So, I don't see how the errors you say you are getting have anything to do with your database. If you want further help, please provide details.
-
Um, yeah. But, why spend time trying to get an old, deprecated process to work when the process will likely be removed in the next release of PHP. I would guess most people are leaning towards PDO these days (I do). But, you can leverage what you have already written to use the mysqli_ extensions with very little effort. As to your problem, saying "it doesn't work" provides us no information to work with. What errors are you getting? What is or is not being displayed? What have you tried to resolve the problems and what were the results?
-
A few other notes: if ($_FILES[csv][size] > 0) { Textual array indexes should be enclosed in quotes. From the manual Also, the code uses a do {} while() loop. In the code block the code is looking to perform work against the variable $data. But, that variable isn't defined in the first execution of the loop. So, the first iteration of the loop is a waste. It should instead use just a while() {} loop since you want to define $data before the first execution. (EDIT: This is the reason you had to add a if ($data[0]) in that loop. With a while() loop, you don't need to do that check since you know that $data would always be defined on each iteration of the loop) Lastly, to add to Jacques1's response regarding the use of 'YES'. When you do use Booleans, it makes the code simpler. If you set $errors = FALSE; as the default and then set it to $errors = TRUE; when you detect an error, the condition to check if there were errors can be written simply as if($errors) {}. No need to write if($errors==TRUE) {}. A condition 'passes' if the results of the condition result to TRUE. So, if a value will only be TRUE/FALSE you don't need to create a comparison for the condition.
-
He said it didn't. Since he was posting here for help stating that it wasn't allowing an empty value, I took him at his word. I stated that the expression I provided was a hack. With the assumption that the original expression didn't work, I tested this modified one to verify it returned the requested results.