Jump to content

DavidAM

Staff Alumni
  • Posts

    1,984
  • Joined

  • Days Won

    10

Everything posted by DavidAM

  1. It does kind of feel that way, doesn't it. I mean, after all, there is a print statement before the header call. But if you follow the flow of the logic; when that print statement is actually executed, you will never get to the header call.
  2. @ChristianF: Yeah. I'm going to have to modify my signature. I often copy the OP's code and make minor changes. I figure they will recognize the code easier to see where the changes need to be. Aside from moving that IF statement, the only thing I did was to add the exit call; that particular change is (or can be) critical and I thought it necessary to include it. I do not feel the need to completely rewrite the code. If the OP doesn't recognize his/her code, they may have trouble incorporating the changes or just doing a copy-paste without understanding what change they are making. Either way, it does little good for anyone (IMHO).
  3. The reason the user gets inserted is because that's what you told it to do. The INSERT statements are outside of the if test. So, the INSERT will always be executed. Also: Where are you getting $user, et. al. from? That looks suspiciously like register_globals is on. THIS IS A BAD THING. Even with register_globals OFF, you need to trim those fields. Otherwise, a user can just enter spaces. So, it should be: if (trim($_POST['user']) == '') { #error condition Also: You are SELECTing the id from the users table when checking for an existing username or email address. But you are then checking the user column and the email column, neither of which is there. You are also double-fetching the results which will put you past the end of the result set if there is only one result, and you are not checking all of the results if there are more than 1 (uhh, I mean 2). $query = "SELECT id FROM users WHERE user = :user Or email = :email"; # # ... snip ... # $row = $stmt->fetch(); if ($row = $stmt->fetch($result)){ if ($row["user"] == $user) { $errors[] = "Your username is already used by another member."; } if ($row["email"] == $email) { $errors[] = "Your e-mail address is already registrated in our database."; } } Work on your indentation. It will make the code much easier to read. I completely missed the fact that validadres is inside of the if block. In fact, you may not have meant to put it there in the first place. You are asking the user for a lot of information. And then you die if any of it is bad. This means they have to find their way back to the registration page, and re-type everything. Since they have no control over what user names are already in use, this will get frustrating and cause people to give up trying to register, loosing you potential users. Learn about "sticky fields". Basically, you want the form and processing in the same script, so you can reshow the form along with any error messages and all of the data that the user already entered. I'm not hating on your code. I have made every one of these errors at one time or another. Actually, I'm kind of glad my first development server crashed and lost the hard drive. I'll never again have to look at the garbage I wrote when I was learning PHP.
  4. The if ($send) ... needs to be inside of the else on the if ($from .... Otherwise, you are sending a message which your code says is invalid. if($from == '') { print "You have not entered an email, please go back and try again";} } else { $send = mail($to, $subject, $body, $headers); $send2 = mail($from, $subject2, $autoreply, $headers2); if($send) { header( "Location: http://www.websites.cx/indextest.html" ); exit(); ## ALWAYS INCLUDE AN EXIT AFTER A HEADER Redirect. Otherwise, the PHP engine will continue executing code. } else { print "We encountered an error sending your mail, please notify [email protected]"; } }
  5. History Lesson: The \r is a Carriage Return (CR) and the \n is a New-Line (or Line Feed (LF)). The use of two separate characters comes from the time when, believe it or not, there were no video monitors on computers. The computer terminal was basically a typewriter. And if you think about a typewriter, this next part makes sense. The CR (Carriage Return) sent the carriage that was holding the paper back to the beginning of the line. The LF (Line Feed) shifted the paper up to start printing on the next line. They needed two separate commands because the only way to get BOLD print and some special characters was to print on the same line a second time. So you could send a CR to go back to the beginning of the line and start sending characters again to type on the characters that were just typed. If there were no special characters or bold printing to do, the two commands were sent one after the other -- CR LF -- at the end of a line. MS kept the two-character "End-of-Line" sequence (CRLF) for text files. Unix uses a single character (LF). I think Unix used a single character to save bandwidth in a time when 1200 baud modems were the norm. There was a time (I believe) when another OS used only (CR) as an end-of-line character. In general Windoze will send CRLF when the user types in a TEXTAREA (at least I.E. used to do that). The only real reason to keep the two-character sequence is if you will be writing to a text file and sending that file to someone who will open it on Windoze. Even then, most text editors will handle the single-character EOL (MS Notepad will not). So, to keep things simple in your application, you can replace the CRLF pair with LF. So, the code given by ChristianF could be enhanced to: $replace = array ("\r\n", "\r", "\n"); $descrip = str_replace($replace, "\n", $descrip); echo '<p>'.str_replace ("\n", "</p>\n<p>", $descrip).'</p>'; Notes: In this case, you must use the double-quotes -- the control characters will not be interpreted in single quotes. The order of the entries in the array is important. If the CRLF pair does not come first, each will be replaced separately. Also, str_replace is recursive. That is, the replace is done for each element of the array. This is why I added the extra line of code above. The code would be equivalent to: $replace = array ("\r\n", "\r", "\n"); foreach ($replace as $char) { $descrip = str_replace($char, "\n", $descrip); } As you can see, all CRLF will be replaced with LF. Then all CR will be replaced with LF. Then all LF (including the ones we just put in) will be replaced with LF. If we used a different sequence in the array, and were replacing with "</P>\n<P>" then we could end up with: # What if we used $replace = array ("\r", "\n", "\r\n"); Start: Something\r\nelse 1st Iter: Something</P>\n<P>\nelse 2nd Iter: Something</P></P>\n<P><P></P>\n<P>else 3rd Iter: Something</P></P>\n<P><P></P>\n<P>else Which is clearly not what we wanted. In truth, we could leave the LF out of the $replace array entirely, since we do that replacement separately. $replace = array ("\r\n", "\r"); $descrip = str_replace($replace, "\n", $descrip); echo '<p>'.str_replace ("\n", "</p>\n<p>", $descrip).'</p>';
  6. Read the error message: You have to specify the filename in the directory.
  7. From the PHP manual: next foreach Your call to next() is advancing the pointer that foreach is using. So, yes, it will screw up the process. As Pikachu said, if all you need is the comma-separated list, use implode. If there is more going on in that loop. I typically would use something along the lines of: $my_territories .= (empty($my_territories) ? '' : ', ') . $val;
  8. The caret ("^") at the beginning of the RegExp, anchors it the the start of the string. The dollar-sign ("$") at the end of the RegExp, anchors it to the end of the string. So that expression will only match a string that is composed entirely of digits (one or more). If you take the caret and dollar-sign out, it will match one or more digits anywhere in the string. You will not get any indication of how many total digits are in the string, so it will not work if the requirement is "2 or more digits"; and it will not indicate if all of the found digits are consecutive: i.e. it will match "a1b" and "a11b" and "a1b2". Note that preg_match returns the number of full pattern matches found. But, it will always stop as soon as it finds a match, so the return value will never be greater than one. preg_match_all will find all pattern matches and return the number found. But again, it is the count of the number pattern matches (one or more consecutive digits) and not the count of the total number of digits in the string. i.e. in the examples I gave above, the return values would be: 1, 1, and 2.
  9. To elaborate on the other good answers you have received: It is the nature of a relational database that you will get the same data in some columns many times when querying multiple tables. You can use these columns to know when the "outer" data changes. So, using your schema, if you wanted to print the schedule for ALL teams, you could issue a single query, and start a new HTML table whenever the TeamID changes. Having learned and worked with SQL before the JOIN phrase became a standard, I do not usually push it as the "correct" way to build a query. When I first learned the JOIN syntax, it was confusing; especially when doing OUTER JOINS. However, it does make the query easier to read and makes it harder to leave out a relationship. For instance, if you left out the AND teams.teamid = players.teamid clause in your query, you would get a Cartesian product, which means that every matching row from the rest of the query would match EVERY ROW in the teams table, and you would get back a whole lot more data than you really want. Using the JOIN syntax, it would be very obvious in looking at the query that the relationship is missing -- in fact, the table would be missing from the query, entirely: SELECT * FROM players JOIN positions ON players.positionid = positions.positionid JOIN teams ON teams.teamid = players.teamid JOIN schedule ON (schedule.homeid = players.teamid OR schedule.awayid = players.teamid) WHERE playerid IN ($specificplayerids) ORDER BY lastname"; Do not use SELECT * especially in multi-table queries. SELECT * sends every column from every table to the client (your PHP script) for each selected row. Then PHP has to buffer all of that data internally, and pass it to your while loop in turn. This is a lot of wasted resources (transfer band-width, memory, cpu). Not to mention the fact that columns with the same name (i.e. "positionid") will be "aliased" (i.e. "players.positionid" and "positions.positionid"). If you are using mysql_fetch_assoc you will probably try to reference it as "positionid" and you will find that that index does not exist in the returned array. You may want to add "weeknum" to your ORDER BY. Another "feature" of a relational database is that there is no "natural order" of data. Just because the schedule table was built in the proper date sequence does not guarantee that the data will be returned in that sequence. When joining tables and/or ordering results, the database may actually retrieve the data using some index that does not give it "sequential data". Also, I would add the player's ID to the order by: ORDER BY lastname, playerid, weeknum. Why? because if you have multiple players with the same last name, their rows could be intermixed since there is no "natural order" to data (see above). Then use the "playerid" (not the "lastname") to determine when you start dealing with a different player.
  10. I have to agree, that is an elegant solution. Sorry, I did not get it at first. It is just the kind of solution I look for to avoid IF's and CASE's and looping queries. @mbb87 The reason it is updating rows you don't want it to is because you are not limiting the update to the specific team you are dealing with. Since we haven't actually seen the query that selects the list of players, we can't offer a complete solution. However, some kind of WHERE clause selecting the team in question should work. If the database is normalized, it would require a JOIN to the table that links players to their team. You would have to pass the team ID in a session field (preferred) or a hidden form field (not recommended) between the two scripts. Sorry, I missed the quotes (instead of backticks) on the table name and sent you down the wrong road. For the record, I never use backticks on table or column names. If I accidentally use a reserved word for a (table or column) name, I change the name. While using backticks is permitted (not required); it makes the code more difficult to read (IMO) and leads to problems if you forget them or inadvertently use single-quotes. I just eliminate the potential problems by not using reserved words for names and therefore not needing to use backticks.
  11. Did you read and understand that output? 1) The checked players list is empty. I don't think that "int" is a function. I think that line needs to be $checked_players_list = implode(', ', array_map('intval', $_POST['pl_id'])); 2) Your query is missing a couple of things. It has SET `block` = `players`.`pl_id` IN () but in your earlier posts, it was SET `block` = $block. Then it is missing the "WHERE" -- it should (probably) be $query = "UPDATE 'players' SET `block` = $block WHERE `players`.`pl_id` IN ({$checked_players_list})"; Programming requires some analytical thought. You shouldn't just cut-and-paste suggestions from people without trying to understand what you are pasting. Psycho really gave you the answer, there; it just had a couple of minor issues. If you read his signature you see the disclaimer that any code he provides may not be perfect. The code I provided above may not be perfect. This is not because we are bad programmers. We just don't have the time, and in some cases, the capability, to build a complete environment to fully test your problem. Often we do not have enough information from you to fully build/test the code. We expect you to make an effort to understand and integrate our suggestions.
  12. The Explain Plan indicates it is not using an index; it is doing a table scan. Is that a composite index --- all three fields? Since your WHERE clause specifies only one criteria, and it is on "saleby", that index will not likely be chosen for use. You should turn the fields around so that the "saleby" is first --- saleby, orderid, id. I've heard that mySql stores the PRIMARY KEY in the index anyway, so that column may not be needed -- (I heard that on a forum, but have never confirmed it). How selective is "saleby"? You have over 61,000 rows in that table, are there a significant number of discreet values for "saleby" and are they fairly evenly distributed? My point is, if that column is a boolean, so there are only two values (0 and 1), then it will not be very selective, anyway (about half the table), and may not be useful. Have you OPTIMIZEd the tables or ANALYZEd the tables since loading the data? Adding an index to a table automatically does this, and may be where your improvement came from. ANALYZE rebuilds the distribution data so the engine can make more informed decisions about how well a particular index will help. OPTIMIZE rebuilds the indexes, with the same affect, but also rearranges the index pages to make access faster --- that's a simplified explanation. As Matt suggested, we need more information about the database structure to help determine how to optimize that query. It would also help to know what it is you are trying to retrieve. The name you choose for the index -- "last 5 orders" -- seems to imply that you want recent orders, but the WHERE clause -- saleby = 1 -- seems to indicate something else. Show us the table definitions and tells us what you are after, and we may be able to help.
  13. Content-Type: multipart/mixed; boundary="PHP-mixed-XXXXXX" is a header. Everything else you are putting into $headers after that line is the body of the message. Strictly speaking, the "headers" end when a blank line is found (which is indicated by the $eol . $eol in your code). However, as the PHP mail() function does it's job, it may be adding other header stuff and that may be breaking your structure. It does, after all, have to add the To: and Subject: headers. I'm not sure if you need the extra EOLs you are putting inside the multi-part message (the part I just told you to move out of the headers), I don't think you do. And that may break the base64 encoding (I doubt it, but I would check). Since mail clients are like browsers, in that they seem to make up their own rules for interpreting the content; you should always get as close as you can to the specifications. Or install every mail client known to man and test all of your emails.
  14. Without knowing what approach you took to duplicating the Header, it is difficult to suggest how you can duplicate the lines. One approach is to use SQL to INSERT INTO ... SELECT ... FROM ... to copy the header and then the lines. Be sure to set whatever columns need to be adjusted for a "New" PO (i.e. OrderDate, OrderedBy, Qty Receied, etc.). Then ship the user off to the Edit PO Screen. A different approach is to send the user to the New PO Screen and populate all of the fields from the original PO (with adjustments as we did above) just as if they were editing a PO; but when you save, its an INSERT instead of an UPDATE. One advantage of this process is you don't have PO's in the database if the user decides "nevermind, that's not what I wanted to do". Which approach you take, will depend heavily on how your Create and Edit PO processes exist today and how much they may need to be modified to work with either approach. The first approach should require less changes to the existing system, but may produce more garbage in the database if the user changes their mind before they save.
  15. So instead of using a loop, you could change that one line of code to: $deny_ips = array_map('trim', file('ip.txt')); to solve the problem. Using array_search() is a better solution than a loop. Yes, the function (probably) uses a loop; but that loop is coded in C and compiled, and will run faster than the loop in PHP. This is not a big deal with a short list like you have, but it is a good thing to keep in mind for those times when you are processing array data.
  16. You can try this: SELECT x, y, COUNT(*) AS CountRows, COUNT(DISTINCT alliance) AS CountAlliance FROM planets WHERE x = y GROUP BY x, y HAVING CountRows = CountAlliance Unfortunately, it is not going to give you the alliance value. I may have misunderstood the requirements. This query returns rows where x = y. If you want any combination of x,y where all rows with that combination have the same alliance, you can take the WHERE clause out. This query is going to scan the entire table. So, the more data you have, the slower it will be. (depending on indexes, etc).
  17. Why are you calling me_mysql_prep to send data to the browser? Either that function is for preparing data to send to the database or it is woefully misnamed. You do not ... let me say it again ... you do not need or want to perform database escapes on page content. If you do, you end up displaying carriage-returns and stuff that you don't want to see.
  18. You are selecting data from 6 tables with NO JOINS AT ALL. This will result in a cartesian product. So for each of the 23 rows in courses, you get all 4 of the gs rows, and for each of those you get all 4 of the fs rows, and for each of those you get all 4 of the wind rows ... That's 23 * 4 * 4 * 4 * 4 * 4 rows = 23,552 rows. Which are then sorted randomly, so one row can be returned. You say you call that 12 times. If you really need random rows from a cartesian product, why don't you change the LIMIT to 12, so you only do the table scans and sorts one time? In general, ORDER BY RAND() is not healthy, and will not scale well. That is, if you get 2300 courses (and associated data) in your database, it will run significantly slower than it is now. Cartesian products are also not healthy and don't scale. There is not enough information about your database to offer an alternative, but I suggest you need one.
  19. When you get ready to present this to somebody, leave the "I's" out of it. Sit back and read that list, it sure sounds ego-centric -- not saying you are, I don't believe you are, just saying that the "boss" will likely hear it that way and it takes you out of the team. Make sure the points all provide benefit to the company and/or the boss. Make it a (personal) policy to improve every file you touch, in some way. Not be re-writing it, unless that's what needs to happen. When you figure out what a function or code block is doing, add a documentation block explaining it; at the very least, you won't spend another hour figuring it our three days later when you have to look at that file again. It will also keep you from having that "I didn't accomplish a damn thing today" feeling every day when you leave. It sounds like you're saying every developer has their own database? It's been a while since I was in a team environment, but that just seems odd to me. Having one dev db automatically provides some "testing" against "bad data", since devs are always creating bogus records to test stuff. If there are separate databases, I'd see if someone else has a more recent schema. If not, I'd make friends pretty quickly with someone who could at least dump the create statements from production for me.
  20. First, I don't think this code is doing what you think it is doing. ([paintings]+) -- The parenthesis make it a capturing group so you can refer to the value as $1 in the rewrite. The square brackets make it a character class so any of the letters inside the bracket must be matched: "aginpst" are the letters in your character class (I just removed the repeating ones and alphabetized it). The "+" means 1 or more of the letters (from that class). So you are rewriting artwork/ants/bugs/2 to paintings/acrylics/blue.php?page=2. Second, what distinguishes this rewrite rule from the one you wanted to go to "organge"? They are exactly the same as far as I can see. Third, why are you capturing the first two values, you are not using them in the rewrite. I think you need to rethink what you are trying to accomplish. It might actually solve your question about orange. As for the trailing slash; I usually use something like: RewriteRule ^artwork/paintings/(blue)/([a-z]+)/([0-9]+)/? paintings/acrylics/$1.php?page=$3 [L] Which makes the trailing slash optional for that rewrite. Granted, I'm no expert at this, and I still have problems with the trailing slash from time-to-time.
  21. if WHERE x like '%y%' is a requirement, you will have to use a FULLTEXT index or some other indexing scheme. Anytime you use a wildcard at the start of a LIKE expression, it prevents the server from using an index for that field. So, if this is the only condition in your WHERE clause, the server will have to read EVERY row in the table. If you can remove the first wildcard (like so): WHERE x like 'y%' then you can create an index on "x", and the server will (have the option to) use it to read fewer rows. You might consider re-designing your database. Anytime you have non-meaningful names for columns, including numbered column names for "repeating" data; it generally means the database is not normalized. Contrary to what many think, fewer tables does not necessarily make a better database. If you can normalize that database, you could more than likely get better performance, even with 24 times as many rows.
  22. You got the "localhost/" on it because without the leading slash (on "domain.com") the browser thinks it is a relative path, and automatically applies the path of the current page. At the very least, you need a slash at the beginning: /domain.com. However, I would instruct the user to include the protocol ("http:") and reject the submission if it is not present. It is possible that the user's protocol is https:, so you really want to have them enter it, rather than guess. I would resist storing or showing it without the protocol, because if you show it through some other protocol (i.e. rss or some future protocol), you don't really know what that (future) reader will do to unqualified addresses.
  23. While I don't like sites that enforce such arbitrary limits, you would need two separate character classes since the last character restriction is different from the restriction for all other characters: ^[0-9a-zA-Z_ -]*[0-9a-zA-Z]$ However, I would think you want to prevent a space as the first character as well: ^[0-9a-zA-Z_-][0-9a-zA-Z_ -]*[0-9a-zA-Z]$ Oops. That would require (allow) a minimum of two characters (in the second example), since the middle restriction is optional (0 or more). If you want to force a minimum and/or maximum, you would set a limit on the middle class with the bounds being (in the second case) two characters less than the requirement --- because the first and last must match one character, each. So to force 8 to 20 characters, it would be: ^[0-9a-zA-Z_-][0-9a-zA-Z_ -]{6,18}[0-9a-zA-Z]$ Note: I am not an RegExpert, so I'm sure someone else may have a better idea. Note: This is untested. I leave that as an exercise to the reader.
  24. I really wish you people would read the error message in the very first post!! Whether or not the query is valid, is of no consequence at this point! The OP does not have an open database connection. Are you kidding?! The error says "object is closed". It does not say "error in query" That's ridiculous. Everyone has been giving advice that is not related to the problem. It does not matter how perfectly he/she builds the query, or how many different ways he/she writes it, if there is no database connection, he/she will continue to get the error because "the object is closed". @perplexia I applaud your stick-to-it-ive-ness (or whatever). Your action by moving the code shows a real desire to fix the problem. You just left out one step, as jcbones pointed out (which he originally did at Reply #18) but was probably missed in all the static about your incorrect query. Here is the code you posted most recently --- notice my comment ### COMMENT ###: $databaseLocation = "c:\\wamp\\www\\Jim_Langeveld\\Maintenance1.mdb"; //Dreate an instance of the ADO connection object $conn = new COM ("ADODB.Connection") or die("Cannot start ADO"); //Define connection string, specify database driver $connStr = "PROVIDER=Microsoft.Jet.OLEDB.4.0;Data Source=$databaseLocation;"; ### COMMENT ### ### YOU NEED TO NOW USE THE connect string IN A CALL TO $conn->Open(); ### COMMENT ### //get the main menu and display it at the top of the page include('menu.html'); //insert the data $insert = "INSERT INTO tblPurchasesHeader (Vendor, InvoiceDate,Invoice,TotalAmount,TaxAmount) VALUES (".$_POST['vendor']."','".$_POST['date']."','".$_POST['invoice']."','".$_POST['total']."','".$_POST['total']."')"; //Insert the new data into the database $conn->Execute($insert); You say you have another script that is accessing the database. Have a look at it, you should be able to find a Connect() or Open() method call. (I actually think it is "Connect" but I haven't used ADODB in code in a while, and don't have any of those projects available to review right now. Note: I am not saying that there are not problems with your code. As ChristianF has posted while I was composing this message, you need, (must) sanitize your inputs before putting them into an SQL string.
  25. That first line should be $users_query = mysql_query(users_sql) or die (mysql_error());
×
×
  • 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.