Menu
Community /Pin to ProfileBookmark
@NogDogMar 20.2008 — #Well, I can't exactly call that "11 lines of code" since I have no idea how many lines of code within the "basket" class are involved in this process.
A few comments for what is there:
Where is the validation that this is a legal value, both in terms of format and an actual item that you have for sale?
If $basket->items[$iteim] does not yet exist, then this will throw a notice-level warning. This might be better handled by a method that initializes the array element value to 1 if not yet set, else increments it if it does. That method might also include the validation mentioned above. If so, then you would check the return value of the method rather than simply setting the object variable, and if it returns false (or whatever error code you want to use), then you would return an error message to the user or such. Generally I prefer to avoid direct access of object variables, declaring them as private (or protected) and requiring that all access go through a method, limiting the damage that careless setting of those variables could do.
The good news is that you are using object-oriented programming to help keep your code modularized, which [i]does[/i] keep this portion of the code relatively simple and clean. The bad news is that the "sunny day scenario" stuff is usually the easy part. It's accounting for all the things that could possibly go wrong that starts to increase the "code bloat".@NogDogMar 21.2008 — #Generally speaking, the "stress" of reading and executing a few more lines of code is trivial compared to the entire server load. I'm more than willing to sacrifice a couple milliseconds to ensure that my application is correct and secure. Your most likely source of [i]noticeable[/i] delays are going to be network and database issues. I've written PHP applications that have to load 4-5 class definition files, maybe a configuration file and a login access file, then execute the actual page including a couple database queries, and it responds quickly enough that you can barely tell the difference versus a plain, single-file HTML page with a couple modest-sized graphics.
I'd rather keep the input validation within the application scripts, as (1) I don't know that you can count on Apache's regex checking to be any faster tha PHP's, and (2) it then leaves you open to problems should the .htaccess file be changed, deleted, corrupted, moved, etc. by accident or on purpose. Also (3, I guess?) it allows for more dynamic error checking and ease of modification should your validation requirements change.@NogDogMar 21.2008 — #
It depends, of course. ? If the value must be non-negative integer, you could use the ctype_digit() function to test it for your initial validation. At some point in the process you'll probably need to verify that it exists in the database. This might be part of the job of a BasketItem class, to which you would pass the ID to its constructor, and it would populate itself from the database with applicable data, or throw an exception if the ID does not exist.
Well, "best practice" tends to be in the mind of the beholder. The MVC people will tell you that you must design your application in such-and-such way, and the database must be designed like so, etc. The XP crowd will tell you to build the simplest thing that does the job, to code using "test first" principals, etc.
Matt Zandstra's [i][url=http://www.apress.com/book/view/9781590599099]PHP 5 Objects, Patterns, and Pracice[/url] [/i] does a good job of suggesting a lot of best practices for building PHP applications, but it does not get down into any nuts-and-bolts stuff about specific algorithms or functions to use.
complex basket tutorials! WHY?
I am currently in the middle of building an e-commerce system for my final year project at university and in my second year I can remember spending hours tapping away with tutorials I found on the internet that all seemed to require huge amounts of what I now see an unnecessary code.
This is the add to basket script i wrote this morning.
[code=php]<?php
require_once ‘includes/class_basket.php’;
session_start();
$basket = new basket($_SESSION[‘basket’]);
if ($_GET[‘item’])
{
$item = $_GET[‘item’];
$basket->items[$item]++;
$_SESSION[‘basket’] = $basket->items;
header(“Location: “.$_SERVER[‘HTTP_REFERER’]);
exit;
}
?>
11 lines of code (if you can even call it 11 lines).
It seems too good to be true is there anything critically flawed with my technique?
P.S. The value of $_GET[‘item’] is a unique product code to size and colour of the item
Sign in
to post a comment6 Comments(s) ↴
0
@NogDogMar 20.2008 — #Well, I can't exactly call that "11 lines of code" since I have no idea how many lines of code within the "basket" class are involved in this process.A few comments for what is there:
[code=php]
$item = $_GET['item'];
[/code]Where is the validation that this is a legal value, both in terms of format and an actual item that you have for sale?
[code=php]
$basket->items[$item]++;
[/code]If $basket->items[$iteim] does not yet exist, then this will throw a notice-level warning. This might be better handled by a method that initializes the array element value to 1 if not yet set, else increments it if it does. That method might also include the validation mentioned above. If so, then you would check the return value of the method rather than simply setting the object variable, and if it returns false (or whatever error code you want to use), then you would return an error message to the user or such. Generally I prefer to avoid direct access of object variables, declaring them as private (or protected) and requiring that all access go through a method, limiting the damage that careless setting of those variables could do.
The good news is that you are using object-oriented programming to help keep your code modularized, which [i]does[/i] keep this portion of the code relatively simple and clean. The bad news is that the "sunny day scenario" stuff is usually the easy part. It's accounting for all the things that could possibly go wrong that starts to increase the "code bloat".
0
@NogDogMar 21.2008 — #Generally speaking, the "stress" of reading and executing a few more lines of code is trivial compared to the entire server load. I'm more than willing to sacrifice a couple milliseconds to ensure that my application is correct and secure. Your most likely source of [i]noticeable[/i] delays are going to be network and database issues. I've written PHP applications that have to load 4-5 class definition files, maybe a configuration file and a login access file, then execute the actual page including a couple database queries, and it responds quickly enough that you can barely tell the difference versus a plain, single-file HTML page with a couple modest-sized graphics.I'd rather keep the input validation within the application scripts, as (1) I don't know that you can count on Apache's regex checking to be any faster tha PHP's, and (2) it then leaves you open to problems should the .htaccess file be changed, deleted, corrupted, moved, etc. by accident or on purpose. Also (3, I guess?) it allows for more dynamic error checking and ease of modification should your validation requirements change.
0
@NogDogMar 21.2008 — #Would you just use a Regular Expression to check the format of the code or would it more advised to run a query as an item is added to the basket to check the product is in the database?[/quote]
It depends, of course. ? If the value must be non-negative integer, you could use the ctype_digit() function to test it for your initial validation. At some point in the process you'll probably need to verify that it exists in the database. This might be part of the job of a BasketItem class, to which you would pass the ID to its constructor, and it would populate itself from the database with applicable data, or throw an exception if the ID does not exist.
The problem I have is that there are endless locations to find technical help with PHP but I am yet to find anywhere that you can find out about best practice from.[/quote]
Well, "best practice" tends to be in the mind of the beholder. The MVC people will tell you that you must design your application in such-and-such way, and the database must be designed like so, etc. The XP crowd will tell you to build the simplest thing that does the job, to code using "test first" principals, etc.
Matt Zandstra's [i]