KevinM1
-
Posts
5,222 -
Joined
-
Last visited
-
Days Won
26
Everything posted by KevinM1
-
Instead of making $message public, keep it private and make a new method that returns the result of $message->messageStatus. Something like: public function isValid() { return $this->message->messageStatus(); } So, you'd use if (!$user->isValid()) { // error code } Much more concise, and it leaves your encapsulation intact.
-
For ajax code examples, you should look at the two jQuery links I gave you earlier in the thread. jQuery is nice in that all of the complicated ajax code is hidden from view, within the depths of the framework. This will allow you to really see what ajax does without getting interfered with by technical details. You won't find easier code examples on ajax.
-
Look at my edit above.
-
Like I said before, it's okay to have display logic (read: little bits of PHP that are only used to output values) within your HTML. PHP that actually processes data should be placed above your HTML (or, in larger systems, within separate files altogether). Look into Separation of Concerns and the MVC pattern. EDIT: Okay, your User is automatically spitting out error messages, which is why you're getting a <ul> before your HTML officially begins. You'll need to rework it so you output errors within your HTML. To do this, you'll need to figure out a way to hold onto your errors before outputting them. Instead of having your User contain a FormValidator, you may be better off passing the User to FormValidator as a parameter to a validation method. Something like: <?php $validator = new FormValidator(); $tmpUser = User::create(/* form values */); // look into static methods and the Factory Method pattern if ($validator->isValidUser($tmpUser)) { $tmpUser->login(); } else { $errors = $validator->getErrors(); } ?> <!doctype html> <html> <!-- head tag, and all that jazz --> <?php if (isset($errors)) { echo $errors; } ?> <!-- more HTML --> </html>
-
I can't give you the best course of action without seeing where your messages are formed, and where they're being outputted from.
-
Problem 1: That will work. In fact that's how some existing sites do it. Problem 2: You really need to read (or re-read) what I wrote: I really can't make it more clear than that. --- Changing gears a bit, you should never blindly accept incoming data of any kind. This is especially true if you're using that data to decide which page to include. You should have an array of allowed file names and you should check your $_GET['page'] actually contains one of those names.
-
All your PHP code should be above your HTML. Where is your <ul> coming from? Also, I can't read Dutch, so I can't tell what that article is saying. What $_SERVER['Request_'Method] does is return what request method (GET or POST) was used in accessing your script. Your current if-conditional simply says: "If this was accessed via POST, then handle the form". That really isn't any different than asking whether or not $_POST['submit'] is set. Why? Because $_POST['submit'] will only exist if it was POSTed to the script anyway. Like I said before, checking whether the submit button was pressed doesn't do much aside from providing one more value to check. I personally like checking the button because it's a bit shorter to write, it fits the standard pattern, and it gives someone trying to get into my system an extra input to account for. Not a lot of security help, but I'll take what I can get. And again, to be abundantly clear, I'm not trying to suggest it's a change that will have an appreciable impact on your site security. I'm mostly trying to point out that using $_SERVER['Request_Method'] in this manner does nothing to make your site more secure. For security, real security, you need to: Validate and sanitize all incoming data, which means checking each form field not only contains a value, but contains a properly formed value. So, if a field requires a number, you need to verify it only contains numeric characters, or if you're storing string data in the db, you may want to check for potentially harmful characters like ';'. This is where regular expressions come into play. You need to run any string data that is structurally valid through your database's escape function (like mysql_real_escape_string) to combat SQL injection attacks. You should also setup an anti-forgery token (google cross-site request forgery for more info). Using $_SERVER['Request_Method'] won't make your site more secure.
-
The standard pattern for all PHP apps is to process first, then output. So, here's what you do: 1. Remove your autoload code from being hard coded in this page. Instead, put it in a file and include or require it. This will allow you to have autoload in all of your pages with only one line of code rather than needing to write the entire thing over and over. 2. Change your if-conditional from: if ($_SERVER['REQUEST_METHOD'] == 'POST'){ to: if (isset($_POST['submit'])){ After giving your submit button the name of 'submit'. Right now, you're only checking if any POST data was sent, and not if the submit button was actually pressed. It doesn't do much for security, but it is slightly more restrictive than what you currently have. 3. Validate and sanitize your inputs. Never trust incoming data, especially if it's supplied by an end user. 4. Like I said above, process first, then output. So, don't place ANY HTML above your PHP. This includes your doctype declaration, and all the rest. You want: <?php /* all of your non-display PHP code here, * including your inclusion of your autoload * and all of your form handling code * with that said, here's the common pattern * for sticky forms */ if (isset($_POST['submit'])) { // all form handling here if (/* all inputs are valid */) { // try saving data if (/* everything saved correctly */) { // redirect user if everything is saved correctly } else { // error - display on the form } } else { // error - display on the form } } /* Notice there's NO else clause. * Why? Two reasons: * 1. This will make the form be displayed if the submit button WASN'T pressed (the if-conditional failed) * 2. This will make the form be displayed if there are any errors. An else clause would block that */ ?> <!DOCTYPE <!-- ... --> > <!-- All the rest of your HTML --> The only PHP you should have in your HTML is code which will redisplay input values. For that, do what JKG said, and check if they're set before attempting to output them. Why do it this way? There are a couple of reasons. First, it makes your code easier to write, read, and understand. Second, it saves you from those pesky "Headers already sent" errors when you try to use a header redirect or sessions. Since you're likely going to redirect the user when the form submission is successful (it's not like they'll need to see the form again), this makes your job a lot easier.
-
Databases are queried via user input all the time. What do you think a user registration form ultimately does? Or when data is retrieved based on a GET value? The key is in sanitizing the input. To the OP: If you don't want to use ajax to stop the page from refreshing, just use a PHP session. Sessions can work even when it's just one page refreshing over and over. Then it's just a matter of: var myVar = <?php echo $_SESSION['myVar']; ?>; For ajax, hardly anyone writes raw ajax any longer. Take a look at jQuery's ajax functions, especially $.get() and $.post().
-
You don't need ajax for this. You've just shown that you can dynamically include data based on an incoming $_GET value. What you want to do with your cart is very similar to that. You want to update your HTML based on the request sent into your system. At the top of your script, you need to capture the request data, see what it contains, and then branch off into the right function(s). Again, you've already done this above with your includes being based on incoming GET data. I'm getting the feeling you don't actually understand what ajax is, so here's a quick explanation. Ajax is short for Asynchronous JavaScript and XML. It's asynchronous since it can send HTTP requests to the server, and capture the responses to those requests, without requiring the browser to refresh the current page. JavaScript is the language which does this communication and captures the responses. XML is a bit of a misnomer. Originally, responses were to be little bits of XML. Today, XML isn't used much as a response. Instead, JavaScript Object Notation - JSON - is the response format of choice. That said, anything returned by the server is a response, which is why your raw HTML was suddenly showing up when you didn't want it to. So, in a nutshell, ajax is a bit of JavaScript code that sends requests to some server side code (in our case, PHP). The PHP code executes and sends back a response. The JavaScript code then captures that response for later use (such as sticking it all in a <div>, which is what you want to do). It's not all that different from the normal HTTP request cycle. The biggest differences are that JavaScript makes the requests for you, and since it captures the response, you can dynamically place that data anywhere on the screen without a refresh. So, for absolute clarity, all ajax will do for you is stop the page from completely refreshing when a HTTP request is made. The same, boring GET and POST requests are sent to the server when you use ajax. The process, behind the scenes is exactly the same. This is why I want you to focus on simply getting the PHP to work. If you can get your script to successfully add/remove/edit your cart without ajax, it won't take much to add ajax to the process. I think I've said about all I can, short of actually writing your code for you. Focus on your PHP cart. Remember: you can include your cart based on a request value, then run the appropriate cart function with that value, and, finally, display the results.
-
You're also not guaranteed that HTTP_REFERER will work, or contain the value you want: http://php.net/manual/en/reserved.variables.server.php
-
can anyone see what is wrong with this script?
KevinM1 replied to darkmonk's topic in PHP Coding Help
Don't use w3schools. It's filled with misinformation. The PHP site itself has great examples. -
input1 means nothing within the context of that function. Use this.parentNode.removeChild(this)
-
Happy birthday, gramps!
-
Connections are closed at the end of script execution automatically. This generally means at EOF. For more, see: mysql_close
-
Do you have any specific questions? Or anything you're lacking confidence in? Good rule to follow: never use the 'global' keyword.
-
You've made it too inflexible. Use an array, and pass in the array index through the argument list: function URLs($name) { $urls = array('some key' => 'http://www.yahoo.com', 'another key' => 'http://www.google.com', 'yet another key' => 'http://www.phpfreaks.com'); if (array_key_exists($name, $urls)) { return $urls[$name]; } } Of course, there are still better ways to do this where your allowed URLs aren't hard coded in the function.
-
Don't resurrect dead threads.
-
Like AC said, each method has to return an object for the next method to act upon. Typically, it's the current object ($this) since the methods are acting on it in succession.
-
Yeah, that would work. However, just to be clear, it makes sense to subclass Exception if you need to track what kind of exception is being thrown, or if certain exceptions need more information to be recorded.
-
Just to be absolutely clear, something like (pseudo-code): class Autoloader_Exception extends Exception{} class Autoloader { public someFunction() { if (/* some bad condition */) { throw new Autoloader_Exception(); } else { // continue } } } Is just fine. Also, Exception isn't an exception handler. Exception simply encapsulates error data. They're handled in try-catch blocks.
-
So, why is your autoloader extending that? You don't need inheritance if you simply want to use another object.
-
Also, why are you using inline JavaScript with jQuery? The whole point of jQuery is to be unobtrusive. That's why its robust CSS selector engine is there.
-
What does your PHP script look like? How are you using your ajax? I thought you were going to get rid of ajaxpage() and move on to jQuery? In any event, you need to modify your PHP script in order to have it call your writeShoppingCart() function when you add an item. --- Okay, that said, I strongly urge you to stop (if you can) and work on some fundamentals. Your design has some obvious issues which will make it even harder to progress. I'm getting a distinct "I'm going to throw a bunch of code at the wall and hope something sticks" feeling. This isn't a critique of you personally - we've all been there and started off similarly - but your design is, essentially broken. So, some criticism/suggestions: 1. You really need to ditch your ajaxpage() function. Simply put, it doesn't do what you want, and the workaround I showed you is really just a hack. You also don't seem to understand how it works completely, despite the times I've explained it. That could be due to a language barrier issue, but I'm sensing there's more to it than that. There's no reason not to learn some JavaScript and jQuery for real, especially if you're writing an e-commerce app. Throwing a bunch of requests through a 3rd party ajax function you don't fully understand is only going to end in heartbreak. JavaScript is a requirement if you want to be a professional developer. Might as well start learning it. You could solve all of your ajax problems with a couple $.get() calls. 2. Your cart.php script has some potential. The problem there is that I don't think you have a clear idea of how your cart should actually work. You need to sit down and figure out when you want to send output back to the browser and when it's okay to simply update the internal values. Don't be afraid to write new code. I keep getting the feeling you're stuck on a few actions that the cart should perform, and you're just shoving code into them hoping it will work. Make it work, first, and then trim where necessary. And, really, you should focus on getting the PHP script working perfectly before trying to add JavaScript on top of it. You're trying to do it all at once, which is causing confusion. The back end is the most important part. Your HTML and JavaScript should simply be viewed as a skin you place upon your PHP script. 3. A small thing, compared to the others, but learn Separation of Concerns (SoC), Don't Repeat Yourself (DRY), and Unobtrusive JavaScript. It will make your life so much simpler in the long run if you understand and follow those principles. It will lead to much better written code. Google each of those terms, and really try to understand them. I'm not saying this to discourage you, only to point out that you're going down the wrong road with your design. Take a deep breath, forget about the ajax component, and work on getting the PHP part of it as good as it can be.
-
You're looking at it backwards. You should extend Exception when you want to add functionality to Extension. If you make a custom SQL_Exception class, it should extend Extension, not whatever class you're using to represent your database. The database class can then throw your new SQL_Exception, which can be caught later down the line. A database, a form, an autoloader... none of these are exceptions in and of themselves, nor should they be. The most they should do is throw custom exceptions that relate specifically to them.