Re: [PHP] Optimizing Help
Jochem,
I cannot believe you spent this much time helping me with this. I really
appreciate it. I will go through and make ALL those changes. I am
grateful. Especially the changes which cleaned up jibberish and
tightened from several lines to one - really! Thank you thank you thank you!
--JJ
Jochem Maas wrote:
jack jackson wrote:
Hello,
Recently with the help of several of you I got a script working. It's
complex, I'm still new, some parts of it came from other authors and I
adapted it, and generally despite the fact that it seems to work
perfectly at this point, I am certain that there is bloat, repetition
and simply useless crap.
I'd like to know if anyone can have a look and help me optimize what I
have. For example, there are several mysql queries - would it be
possible to combine them somehow? I use the image upload section twice
because I couldn't figure out how to make it work using different vars
within the same section. That kind of newbie stuff.
The code is here: http://hashphp.org/pastebin.php?pid=3701
# ?php
#
# require_once('nscfg.php');
#
# //Initialize vars
# $query = '';
# $dropdown = '';
# $result = 0;
# $errors = array();
# $msgArray = array();
# $msgText = '';
be neat, it helps:
$query = '';
$dropdown= '';
$result = 0;
#
# $query = 'SELECT art.*,publisher.*
# FROM art,subject,publisher,series
# GROUP BY publisher_name';
make good (ab)use of alignment (your style may differ ;-)
$query = 'SELECT art.*,
publisher.*
FROM art,
subject,
publisher,
series
GROUP BY publisher_name';
// ps - why is 'series' 'subject' in there?
#
# $subject_query = 'SELECT art.*,subject.*
# FROM art,subject
# GROUP BY subject_name';
#
# $series_query = 'SELECT art.*,series.*
# FROM art,series
# GROUP BY series_name';
#
# $media_query = 'SELECT media_id,media_name
# FROM media
# GROUP BY media_name';
#
# $result = mysql_query($query);
# $media_result = mysql_query($media_query);
# $subject_result = mysql_query($subject_query);
# $series_result = mysql_query($series_query);
#
instead of creating 4 result identifiers, run and loop thru 1 at a time,
its allows you to reuse a single $result var (as well as being better in
other ways)
# $dropdown_publishers = 'select name=publisher';
# while ($rows = mysql_fetch_assoc($result)){
# $dropdown_publishers .= 'option value=' . $rows['publisher_id']
. '' . htmlentities($rows['publisher_name']);
# }
# $dropdown_publishers .= '/select';
here is a candidate for a function:
buildOptionList($name, $result, $valKey, $nameKey)
{
while ($row = mysql_fetch_assoc($result)) {
$dropdown[] = 'option value='
. htmlentities($row[$valKey], ENT_QUOTES)
. ''
. htmlentities($row[$nameKey], ENT_QUOTES)
. '/option'; // options have closing tags
}
return 'select name=' . $name . '' .
join('',$dropdown) . '/select';
}
#
#
# $dropdown_subject = 'select name=subject';
# while ($rows = mysql_fetch_assoc($subject_result)){
^- white space?
# $dropdown_subject .= 'option value=' . $rows['subject_id'] .
'' . htmlentities($rows['subject_name']);
# }
# $dropdown_subject .= '/select';
#
# $dropdown_series = 'select name=seriesoption value=Select A
Series/option';
# while ($rows = mysql_fetch_assoc($series_result)){
# $dropdown_series .= 'option value=' . $rows['series_id'] . ''
. htmlentities($rows['series_name']);
# }
# $dropdown_series .= '/select';
#
# $checkbox_media = array ();
# $media_types = array();
# while ($media_rows = mysql_fetch_assoc($media_result)){
# $checkbox_media[] = input type='checkbox' name='media_types[]'
value='{$media_rows['media_id']}' /{$media_rows['media_name']} ;
# }
#
# ///IMAGE SECTION - MAIN IMAGE (IMAGE UPLOAD SCRIPT COPYRIGHT INFO IN
CFG FILE)
# ///THIS SECTION Copyright (c) 2000 Marcus Kazmierczak, [EMAIL PROTECTED]
# if ($REQUEST_METHOD == POST)
# {
# $uploaddir = images/jpg;
#
# /*== checks the extension for .jpg or .tiff ==*/
# $pext = getFileExtension($imgfile_name);
TURN OFF register_globals, and access the $_FILES array instead.
#
# $pext = strtolower($pext);
#
#
# //If main image don't match extentions
# if (empty($POST['imgfile'])) {
# $errors[] = 'Please select an image. This is sort of the whole
point, yes?';
your error checking for whether the image was actually uploaded is
non-existent,
check the manual (+usernotes) for guidelines on how to check whether
anything
made it thru.
# }
#
# if (($pext != jpg) ($pext != jpeg) ($pext != tif)
($pext != tiff))
# {
or maybe:
$validExtensions = array(jpg,jpeg,tif,tiff);
if (!in_array($pext, $validExtensions)) { /* bad */ }
/*
having said that the file extension doesn't say anything - better to use
the
[PHP] Optimizing Help
Hello, Recently with the help of several of you I got a script working. It's complex, I'm still new, some parts of it came from other authors and I adapted it, and generally despite the fact that it seems to work perfectly at this point, I am certain that there is bloat, repetition and simply useless crap. I'd like to know if anyone can have a look and help me optimize what I have. For example, there are several mysql queries - would it be possible to combine them somehow? I use the image upload section twice because I couldn't figure out how to make it work using different vars within the same section. That kind of newbie stuff. The code is here: http://hashphp.org/pastebin.php?pid=3701 Thanks in advance! -- PHP General Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP] Optimizing Help
Hey there Jackson, The first thing I would consider would be to see if you can classify the code into chunks that do a certain job, and then rewrite them as functions. I would also consider looping through arrays for repetitive jobs (lines 258 - 270 for example). Down the line you could consider the use of classes, your own or ready made - such as the ever useful PEAR (http://pear.php.net). I personally found that the greatest saver of time to be the re-use of code that I know works well for given situations. HTH Chris -- PHP General Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP] Optimizing Help
Thanks, Chris, I see what you're getting at. I'm not sure I understand what you mean about looping through the array in the area you specified- isn't that just the definitions of the vars? Or is that what you meant and I'm missing the point!!? On 6/13/05, Chris Ramsay [EMAIL PROTECTED] wrote: Hey there Jackson, The first thing I would consider would be to see if you can classify the code into chunks that do a certain job, and then rewrite them as functions. I would also consider looping through arrays for repetitive jobs (lines 258 - 270 for example). Down the line you could consider the use of classes, your own or ready made - such as the ever useful PEAR (http://pear.php.net). I personally found that the greatest saver of time to be the re-use of code that I know works well for given situations. HTH Chris -- PHP General Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP] Optimizing Help
Jackson, Yeah, what you can do is place all your form values you just got into an array, and then act on each of them in turn by looping through the array when it comes to repetitive jobs, such as the mysql_real_escape_string(trim($_POST['variable'])) part of the code I referred to earlier. You could also use a similar method for generating your date option menus later (lines 342). Needless to say, this is only my way of doing things, and other list members will have different/better ideas no doubt!!! Cheers Chris -- PHP General Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP] Optimizing Help
jack jackson wrote:
Hello,
Recently with the help of several of you I got a script working. It's
complex, I'm still new, some parts of it came from other authors and I
adapted it, and generally despite the fact that it seems to work
perfectly at this point, I am certain that there is bloat, repetition
and simply useless crap.
I'd like to know if anyone can have a look and help me optimize what I
have. For example, there are several mysql queries - would it be
possible to combine them somehow? I use the image upload section twice
because I couldn't figure out how to make it work using different vars
within the same section. That kind of newbie stuff.
The code is here: http://hashphp.org/pastebin.php?pid=3701
# ?php
#
# require_once('nscfg.php');
#
# //Initialize vars
# $query = '';
# $dropdown = '';
# $result = 0;
# $errors = array();
# $msgArray = array();
# $msgText = '';
be neat, it helps:
$query = '';
$dropdown= '';
$result = 0;
#
# $query = 'SELECT art.*,publisher.*
# FROM art,subject,publisher,series
# GROUP BY publisher_name';
make good (ab)use of alignment (your style may differ ;-)
$query = 'SELECT art.*,
publisher.*
FROM art,
subject,
publisher,
series
GROUP BY publisher_name';
// ps - why is 'series' 'subject' in there?
#
# $subject_query = 'SELECT art.*,subject.*
# FROM art,subject
# GROUP BY subject_name';
#
# $series_query = 'SELECT art.*,series.*
# FROM art,series
# GROUP BY series_name';
#
# $media_query = 'SELECT media_id,media_name
# FROM media
# GROUP BY media_name';
#
# $result = mysql_query($query);
# $media_result = mysql_query($media_query);
# $subject_result = mysql_query($subject_query);
# $series_result = mysql_query($series_query);
#
instead of creating 4 result identifiers, run and loop thru 1 at a time,
its allows you to reuse a single $result var (as well as being better in other
ways)
# $dropdown_publishers = 'select name=publisher';
# while ($rows = mysql_fetch_assoc($result)){
# $dropdown_publishers .= 'option value=' . $rows['publisher_id'] . ''
. htmlentities($rows['publisher_name']);
# }
# $dropdown_publishers .= '/select';
here is a candidate for a function:
buildOptionList($name, $result, $valKey, $nameKey)
{
while ($row = mysql_fetch_assoc($result)) {
$dropdown[] = 'option value='
. htmlentities($row[$valKey], ENT_QUOTES)
. ''
. htmlentities($row[$nameKey], ENT_QUOTES)
. '/option'; // options have closing tags
}
return 'select name=' . $name . '' .
join('',$dropdown) . '/select';
}
#
#
# $dropdown_subject = 'select name=subject';
# while ($rows = mysql_fetch_assoc($subject_result)){
^- white space?
# $dropdown_subject .= 'option value=' . $rows['subject_id'] . '' .
htmlentities($rows['subject_name']);
# }
# $dropdown_subject .= '/select';
#
# $dropdown_series = 'select name=seriesoption value=Select A
Series/option';
# while ($rows = mysql_fetch_assoc($series_result)){
# $dropdown_series .= 'option value=' . $rows['series_id'] . '' .
htmlentities($rows['series_name']);
# }
# $dropdown_series .= '/select';
#
# $checkbox_media = array ();
# $media_types = array();
# while ($media_rows = mysql_fetch_assoc($media_result)){
# $checkbox_media[] = input type='checkbox' name='media_types[]' value='{$media_rows['media_id']}'
/{$media_rows['media_name']} ;
# }
#
# ///IMAGE SECTION - MAIN IMAGE (IMAGE UPLOAD SCRIPT COPYRIGHT INFO IN CFG
FILE)
# ///THIS SECTION Copyright (c) 2000 Marcus Kazmierczak, [EMAIL PROTECTED]
# if ($REQUEST_METHOD == POST)
# {
# $uploaddir = images/jpg;
#
# /*== checks the extension for .jpg or .tiff ==*/
# $pext = getFileExtension($imgfile_name);
TURN OFF register_globals, and access the $_FILES array instead.
#
# $pext = strtolower($pext);
#
#
# //If main image don't match extentions
# if (empty($POST['imgfile'])) {
# $errors[] = 'Please select an image. This is sort of the whole point,
yes?';
your error checking for whether the image was actually uploaded is non-existent,
check the manual (+usernotes) for guidelines on how to check whether anything
made it thru.
# }
#
# if (($pext != jpg) ($pext != jpeg) ($pext != tif) ($pext !=
tiff))
# {
or maybe:
$validExtensions = array(jpg,jpeg,tif,tiff);
if (!in_array($pext, $validExtensions)) { /* bad */ }
/*
having said that the file extension doesn't say anything - better to use the
info
returned by imagegetsize() ... read this:
http://nl2.php.net/manual/en/function.getimagesize.php
*/
# print h1DOH!/h1pThat's not a valid image extension. br /;
# print pYou are only permitted to upload JPG, JPEG, tif or TIFF images with the extension .jpg, .jpeg, .tif
or
