Can this code be improoved? If so how?

This is a discussion on Can this code be improoved? If so how? within the alt.comp.lang.php forums, part of the PHP Programming Forums category; Can this code be improoved? If so how? I am new to php. This is one of my first scripts. ...


Go Back   Usenet Forums > PHP Programming Forums > alt.comp.lang.php

Reload this Page Can this code be improoved? If so how?

FAQ Members List Calendar Search Today's Posts Mark Forums Read

Reply

 

LinkBack Thread Tools Display Modes
  #1 (permalink)  
Old 05-27-2005
Michael
 
Posts: n/a
Default Can this code be improoved? If so how?
Can this code be improoved? If so how?
I am new to php. This is one of my first scripts.
Thank you

<?
define("TEXT_SHIP_TODAY", "<BR><B>Place your order today</B> before
<B>4:00pm Est.</B><br>Your order will ship by 5:00pm <B>Today.</B>");
define("TEXT_SHIP_FRITD", "<BR><B>Place your order today</B> before
<B>1:00pm Est.</B><br>Your order will ship by 2:00pm <B>Today.</B>");
define("TEXT_SHIP_TOMM", "<BR><B>Place your order today.</B><BR>Your order
will ship by 5:00pm Est on");
define("TEXT_SHIP_WEEKEND", "<BR><B>Place your order today.</B><BR>Your
order will ship by 5:00pm Est on");
define("HOLIDAY_SHIP_TEXT", "<BR><B>Due to the Holiday.</B><BR>If you place
your order today.<BR>Your order will ship by 5:00pm Est on");
define("TEXT_SHIP_TODAY_FRIDAY", "<BR><B>Place your order today before
1:00pm Est.</B><BR>Your order will ship by 5:00pm Est on");
define("PERD", ".");
$TDAY = date("D");
$TME = date("G:i:s");
$ORDER_WILL_SHIPA = date("l F jS");
$ORDER_WILL_SHIPB = date("l F jS", time()+86400); //86400 = 1 day this is
seconds
$ORDER_WILL_SHIPC = date("l F jS", time()+172800);
$ORDER_WILL_SHIPD = date("l F jS", time()+259200);
$ORDER_WILL_SHIPE = date("l F jS", time()+345600);
$ORDER_WILL_SHIPF = date("l F jS", time()+432000);
$DELAYDAYFL = date("F j");
//Define $IS_SHP_HOLIDAY true
if ($DELAYDAYFL == 'Nov 23' || $DELAYDAYFL == 'May 27' || $DELAYDAYFL ==
'Jul 1' || $DELAYDAYFL == 'Sep 2' || $DELAYDAYFL == 'Oct 7' || $DELAYDAYFL
== 'Nov 8' || $DELAYDAYFL == 'Nov 24' || $DELAYDAYFL == 'Dec 23' ||
$DELAYDAYFL == 'May 28' || $DELAYDAYFL == 'Jul 2' || $DELAYDAYFL == 'Sep 3'
|| $DELAYDAYFL == 'Oct 8' || $DELAYDAYFL == 'Nov 9' || $DELAYDAYFL == 'Nov
25' || $DELAYDAYFL == 'Dec 24' || $DELAYDAYFL == 'May 29' || $DELAYDAYFL ==
'Jul 3' || $DELAYDAYFL == 'Sep 4' || $DELAYDAYFL == 'Oct 9' || $DELAYDAYFL
== 'Dec 25' || $DELAYDAYFL == 'Jan 1' || $DELAYDAYFL == 'May 30' ||
$DELAYDAYFL == 'Jul 4' || $DELAYDAYFL == 'Sep 5' || $DELAYDAYFL == 'Oct 10'
|| $DELAYDAYFL == 'Dec 26' || $DELAYDAYFL == 'Jan 2') {${IS_SHP_HOLIDAY} =
'true';}
else $IS_SHP_HOLIDAY = 'false';
//Define Regular Weekday ship days
if ($TDAY == 'Mon' || $TDAY == 'Tue' || $TDAY == 'Wed' || $TDAY ==
'Thu'){$STDSHIPWD = 'true';}
else $STDSHIPWD = 'false' ;

if ($DELAYDAYFL == 'Nov 23'){${ORDER_WILL_SHIP} = HOLIDAY_SHIP_TEXT . ' ' .
$ORDER_WILL_SHIPF;}
if ($DELAYDAYFL == 'May 27' || $DELAYDAYFL == 'Jul 1' || $DELAYDAYFL == 'Sep
2' || $DELAYDAYFL == 'Oct 7' || $DELAYDAYFL == 'Nov 8' || $DELAYDAYFL ==
'Nov 24' || $DELAYDAYFL == 'Dec 23'){${ORDER_WILL_SHIP} = HOLIDAY_SHIP_TEXT
.. ' ' . $ORDER_WILL_SHIPE . PERD;}
if ($DELAYDAYFL == 'May 28' || $DELAYDAYFL == 'Jul 2' || $DELAYDAYFL == 'Sep
3' || $DELAYDAYFL == 'Oct 8' || $DELAYDAYFL == 'Nov 9' || $DELAYDAYFL ==
'Nov 25' || $DELAYDAYFL == 'Dec 24'){${ORDER_WILL_SHIP} = HOLIDAY_SHIP_TEXT
.. ' ' . $ORDER_WILL_SHIPD . PERD;}
if ($DELAYDAYFL == 'May 29' || $DELAYDAYFL == 'Jul 3' || $DELAYDAYFL == 'Sep
4' || $DELAYDAYFL == 'Oct 9' || $DELAYDAYFL == 'Dec 25' || $DELAYDAYFL ==
'Jan 1' ){${ORDER_WILL_SHIP} = HOLIDAY_SHIP_TEXT . ' ' . $ORDER_WILL_SHIPC .
PERD;}
if ($DELAYDAYFL == 'May 30' || $DELAYDAYFL == 'Jul 4' || $DELAYDAYFL == 'Sep
5' || $DELAYDAYFL == 'Oct 10' || $DELAYDAYFL == 'Dec 26' || $DELAYDAYFL ==
'Jan 2') {${ORDER_WILL_SHIP} = HOLIDAY_SHIP_TEXT . ' ' . $ORDER_WILL_SHIPB .
PERD;}

if ($STDSHIPWD == 'true' && $IS_SHP_HOLIDAY == 'false' && $TME <=
'16:00:00'){${ORDER_WILL_SHIP} = TEXT_SHIP_TODAY . ' ' . $ORDER_WILL_SHIPA .
PERD;}
if ($STDSHIPWD == 'true' && $IS_SHP_HOLIDAY == 'false' && $TME >
'16:00:00'){${ORDER_WILL_SHIP} = TEXT_SHIP_TOMM . ' ' . $ORDER_WILL_SHIPB .
PERD ;}
//weekend shipping
if ($TDAY == 'Fri' && $IS_SHP_HOLIDAY == 'false' && $TME >=
'13:00:00'){${ORDER_WILL_SHIP} = TEXT_SHIP_FRITD . ' ' . $ORDER_WILL_SHIPA .
PERD;}
if ($TDAY == 'Fri' && $IS_SHP_HOLIDAY == 'false' && $TME <
'13:00:00'){${ORDER_WILL_SHIP} = TEXT_SHIP_WEEKEND . ' ' . $ORDER_WILL_SHIPD
.. PERD;}
if ($TDAY == 'Sat' && $IS_SHP_HOLIDAY == 'false'){${ORDER_WILL_SHIP} =
TEXT_SHIP_WEEKEND . ' ' . $ORDER_WILL_SHIPC . PERD;}
if ($TDAY == 'Sun' && $IS_SHP_HOLIDAY == 'false'){${ORDER_WILL_SHIP} =
TEXT_SHIP_WEEKEND . ' ' . $ORDER_WILL_SHIPB . PERD;}
echo $ORDER_WILL_SHIP;
?>


Reply With Quote
  #2 (permalink)  
Old 05-27-2005
Kimmo Laine
 
Posts: n/a
Default Re: Can this code be improoved? If so how?
"Michael" <mids1999@ptd.net> wrote in message
news:S1ydnd6gyMjJXwvfUSdV9g@ptd.net...
> Can this code be improoved? If so how?
> I am new to php. This is one of my first scripts.
> Thank you


Don't get offended, but... It's the single most ugly and discusting code
I've ever seen and I'm sure there are ways improve it. For example an if
statement with over 20 conditions is quite anal and clearly something one
would do with an array.

It lacks comments. I have no idea what it is supposed to do. Adding comments
and explanation would greatly improve it. Re-thinking the structure would
help

$STDSHIPWD = 'true'; ??? What the fuck is this? don't make it a string, just
simply
$STDSHIPWD = true;// or false

Then later when you compare
$STDSHIPWD == 'true' && $IS_SHP_HOLIDAY == 'false'
all you need is
$STDSHIPWD && !$IS_SHP_HOLIDAY etc...

Those were just few things I could think right now. As once said in Beavis
and Butt-head: "This thing sucks more than anything that has ever sucked
before."

(Yes, I understund that it is one of your first codes, and you should think
about getting a book where they teach you the basic ideas of programming -
such as using functions and arrays. The way your doing is like they used to
do it in the 70's and 80's with assembly code. Things have advanced since
those days.)

--
Welcome to Usenet! Please leave tolerance, understanding
and intelligence at the door. They aren't welcome here.
eternal piste erection miuku gmail piste com


Reply With Quote
  #3 (permalink)  
Old 05-27-2005
Daedalus.OS
 
Posts: n/a
Default Re: Can this code be improoved? If so how?
I'm feeling really kind tonight so here is an exemple to illustrate what
kimmo might mean.
Using an array instead of those never ending if condition ... and merging 2
recursive
test into 1. Sometimes a good book is a good friend. Good luck.

<?
/*
Creating an array to hold your holidays with keys set
to the dates an values set to their "WILL_SHIP" code (A trough F)
*/

$HOLIDAYS = array(
'Nov 23'=>'F', 'May 27'=>'E', 'Jul 1'=>'E',
'Sep 2'=>'E', 'Oct 7'=>'E', 'Nov 8'=>'E',
'Nov 24'=>'E', 'Dec 23'=>'E', 'May 28'=>'D',
'Jul 2'=>'D', 'Sep 3'=>'D', 'Oct 8'=>'D',
'Nov 9'=>'D', 'Nov 25'=>'D', 'Dec 24'=>'D',
'May 29'=>'C', 'Jul 3'=>'C', 'Sep 4'=>'C',
'Oct 9'=>'C', 'Dec 25'=>'C', 'Jan 1'=>'C',
'May 30'=>'B', 'Jul 4'=>'B', 'Sep 5'=>'B',
'Oct 10'=>'B', 'Dec 26'=>'B', 'Jan 2'=>'B'
);

/*
Then later in your script the test would look like this
Note that this code is an exemple only ...
I didn't try to rewrite nothing but the 2 loops with little
regards to the rest of the code (Don't use it as is).
*/

if (isset($HOLIDAYS[$DELAYDAYFL])){
$IS_SHP_HOLIDAY = true;
$ORDER_WILL_SHIP = HOLIDAY_SHIP_TEXT .' '.
${'ORDER_WILL_SHIP'.$HOLIDAYS[$DELAYDAYFL]};
}else{
$IS_SHP_HOLIDAY = false;
}

/*
Basically the test use what is contain by $DELAYDAYFL
as the a key for the $HOLIDAYS array. If it is set
(let say $DELAYDAYFL = 'Nov 23'), since $HOLIDAYS['Nov 23'] is set,
this test will pass. Otherwise (let say $DELAYDAYFL = 'Nov 20'),
since $HOLIDAYS['Nov 20'] is not define, the test will fail.

${'ORDER_WILL_SHIP'.$HOLIDAYS[$DELAYDAYFL]}
This mean: Concat the string 'ORDER_WILL_SHIP' and
the value in $HOLIDAYS[$DELAYDAYFL] (wich will be A-F)
and use the result as a variable name...
*/

?>

Dae


"Kimmo Laine" <eternal.erectionN05P@Mgmail.com> wrote in message
news:%Xzle.34$WG5.33@reader1.news.jippii.net...
> "Michael" <mids1999@ptd.net> wrote in message
> news:S1ydnd6gyMjJXwvfUSdV9g@ptd.net...
>> Can this code be improoved? If so how?
>> I am new to php. This is one of my first scripts.
>> Thank you
>
>
> Don't get offended, but... It's the single most ugly and discusting code
> I've ever seen and I'm sure there are ways improve it. For example an if
> statement with over 20 conditions is quite anal and clearly something one
> would do with an array.
>
> It lacks comments. I have no idea what it is supposed to do. Adding
> comments and explanation would greatly improve it. Re-thinking the
> structure would help
>
> $STDSHIPWD = 'true'; ??? What the fuck is this? don't make it a string,
> just simply
> $STDSHIPWD = true;// or false
>
> Then later when you compare
> $STDSHIPWD == 'true' && $IS_SHP_HOLIDAY == 'false'
> all you need is
> $STDSHIPWD && !$IS_SHP_HOLIDAY etc...
>
> Those were just few things I could think right now. As once said in Beavis
> and Butt-head: "This thing sucks more than anything that has ever sucked
> before."
>
> (Yes, I understund that it is one of your first codes, and you should
> think about getting a book where they teach you the basic ideas of
> programming - such as using functions and arrays. The way your doing is
> like they used to do it in the 70's and 80's with assembly code. Things
> have advanced since those days.)
>
> --
> Welcome to Usenet! Please leave tolerance, understanding
> and intelligence at the door. They aren't welcome here.
> eternal piste erection miuku gmail piste com
>


Reply With Quote
  #4 (permalink)  
Old 05-27-2005
Daedalus.OS
 
Posts: n/a
Default Re: Can this code be improoved? If so how?
And since I can't sleep (insomnia probably) here is a little more... using a
simple function

Instead of giving the $HOLIDAYS array values referring to code A through F,
lets give it the number of day we have to add to current date. So the array
would now look like this:
$HOLIDAYS = array(
'Nov 23'=>5, 'May 27'=>4, 'Jul 1'=>4,... // and so on
);

Now lets define a function that will deal with these days:

function shippingDay($days){
$shipDay = date("l F jS", time()+($days*24*60*60));
return $shipDay;
}

Now lets rewrite the loop I posted earlier:

if (isset($HOLIDAYS[$DELAYDAYFL])){
$IS_SHP_HOLIDAY = true;
$ORDER_WILL_SHIP = HOLIDAY_SHIP_TEXT .' '.
shippingDay($HOLIDAYS[$DELAYDAYFL]);
}else{
$IS_SHP_HOLIDAY = false;
}

Later in your script if you would need to get a shipping date, all you need
is to call this function and pass it the number of day to add (
shippingDay(0) would mean today).

Now see how this:
#################################
$ORDER_WILL_SHIPA = date("l F jS");
$ORDER_WILL_SHIPB = date("l F jS", time()+86400); //86400 = 1 day this is
seconds
$ORDER_WILL_SHIPC = date("l F jS", time()+172800);
$ORDER_WILL_SHIPD = date("l F jS", time()+259200);
$ORDER_WILL_SHIPE = date("l F jS", time()+345600);
$ORDER_WILL_SHIPF = date("l F jS", time()+432000);
//Define $IS_SHP_HOLIDAY true
if ($DELAYDAYFL == 'Nov 23' || $DELAYDAYFL == 'May 27' || $DELAYDAYFL ==
'Jul 1' || $DELAYDAYFL == 'Sep 2' || $DELAYDAYFL == 'Oct 7' || $DELAYDAYFL
== 'Nov 8' || $DELAYDAYFL == 'Nov 24' || $DELAYDAYFL == 'Dec 23' ||
$DELAYDAYFL == 'May 28' || $DELAYDAYFL == 'Jul 2' || $DELAYDAYFL == 'Sep 3'
|| $DELAYDAYFL == 'Oct 8' || $DELAYDAYFL == 'Nov 9' || $DELAYDAYFL == 'Nov
25' || $DELAYDAYFL == 'Dec 24' || $DELAYDAYFL == 'May 29' || $DELAYDAYFL ==
'Jul 3' || $DELAYDAYFL == 'Sep 4' || $DELAYDAYFL == 'Oct 9' || $DELAYDAYFL
== 'Dec 25' || $DELAYDAYFL == 'Jan 1' || $DELAYDAYFL == 'May 30' ||
$DELAYDAYFL == 'Jul 4' || $DELAYDAYFL == 'Sep 5' || $DELAYDAYFL == 'Oct 10'
|| $DELAYDAYFL == 'Dec 26' || $DELAYDAYFL == 'Jan 2') {${IS_SHP_HOLIDAY} =
'true';}
else $IS_SHP_HOLIDAY = 'false';
//Define Regular Weekday ship days
if ($TDAY == 'Mon' || $TDAY == 'Tue' || $TDAY == 'Wed' || $TDAY ==
'Thu'){$STDSHIPWD = 'true';}
else $STDSHIPWD = 'false' ;

if ($DELAYDAYFL == 'Nov 23'){${ORDER_WILL_SHIP} = HOLIDAY_SHIP_TEXT . ' ' .
$ORDER_WILL_SHIPF;}
if ($DELAYDAYFL == 'May 27' || $DELAYDAYFL == 'Jul 1' || $DELAYDAYFL == 'Sep
2' || $DELAYDAYFL == 'Oct 7' || $DELAYDAYFL == 'Nov 8' || $DELAYDAYFL ==
'Nov 24' || $DELAYDAYFL == 'Dec 23'){${ORDER_WILL_SHIP} = HOLIDAY_SHIP_TEXT
.. ' ' . $ORDER_WILL_SHIPE . PERD;}
if ($DELAYDAYFL == 'May 28' || $DELAYDAYFL == 'Jul 2' || $DELAYDAYFL == 'Sep
3' || $DELAYDAYFL == 'Oct 8' || $DELAYDAYFL == 'Nov 9' || $DELAYDAYFL ==
'Nov 25' || $DELAYDAYFL == 'Dec 24'){${ORDER_WILL_SHIP} = HOLIDAY_SHIP_TEXT
.. ' ' . $ORDER_WILL_SHIPD . PERD;}
if ($DELAYDAYFL == 'May 29' || $DELAYDAYFL == 'Jul 3' || $DELAYDAYFL == 'Sep
4' || $DELAYDAYFL == 'Oct 9' || $DELAYDAYFL == 'Dec 25' || $DELAYDAYFL ==
'Jan 1' ){${ORDER_WILL_SHIP} = HOLIDAY_SHIP_TEXT . ' ' . $ORDER_WILL_SHIPC .
PERD;}
if ($DELAYDAYFL == 'May 30' || $DELAYDAYFL == 'Jul 4' || $DELAYDAYFL == 'Sep
5' || $DELAYDAYFL == 'Oct 10' || $DELAYDAYFL == 'Dec 26' || $DELAYDAYFL ==
'Jan 2') {${ORDER_WILL_SHIP} = HOLIDAY_SHIP_TEXT . ' ' . $ORDER_WILL_SHIPB .
PERD;}
#######################
Turn into this:
+++++++++++++++++++++
$HOLIDAYS = array(
'Nov 23'=>5, 'May 27'=>4, 'Jul 1'=>4,
'Sep 2'=>4, 'Oct 7'=>4, 'Nov 8'=>4,
'Nov 24'=>4, 'Dec 23'=>4, 'May 28'=>'D',
'Jul 2'=>3, 'Sep 3'=>3, 'Oct 8'=>3,
'Nov 9'=>3, 'Nov 25'=>3, 'Dec 24'=>3,
'May 29'=>2, 'Jul 3'=>2, 'Sep 4'=>2,
'Oct 9'=>2, 'Dec 25'=>2, 'Jan 1'=>2,
'May 30'=>1, 'Jul 4'=>1, 'Sep 5'=>1,
'Oct 10'=>1, 'Dec 26'=>1, 'Jan 2'=>1
);
function shippingDay($days){
$shipDay = date("l F jS", time()+($days*24*60*60));
return $shipDay;
}
if (isset($HOLIDAYS[$DELAYDAYFL])){
$IS_SHP_HOLIDAY = true;
$ORDER_WILL_SHIP = HOLIDAY_SHIP_TEXT . ' ' .
shippingDay($HOLIDAYS[$DELAYDAYFL]);
}else{
$IS_SHP_HOLIDAY = false;
}
++++++++++++++++++++++

And some may have better ways than this one... Anyway I think you get the
picture...
Good learning.

Dae


Reply With Quote
  #5 (permalink)  
Old 05-27-2005
Colin McKinnon
 
Posts: n/a
Default Re: Can this code be improoved? If so how?
Michael spilled the following:

> Can this code be improoved? If so how?
> I am new to php. This is one of my first scripts.
> Thank you
>
> <?
> define("TEXT_SHIP_TODAY", "<BR><B>Place your order today</B> before
<snip>

Yes, there is a lot of room for improvement. I sincerely hope that you are
new to programming.

First, go read a good book on programming (make sure you find out about
variables, arrays, and constants). Then go read the PHP manual. Than go
read the PEAR style rules (at pear.php.net).

It should be possible to write something to do the same thing as your
example with less the 40% of the code you published and make it
understandable.

HTH

C.
Reply With Quote
Reply

« Previous Thread | Next Thread »

Thread Tools
Display Modes
Linear Mode Linear Mode

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts
vB code is On
Smilies are Off
[IMG] code is Off
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are On



All times are GMT +1. The time now is 04:55 PM.

Contact Us - Usenet Forums - Archive - Top

Powered by vBulletin® Version 3.6.8
Copyright ©2000 - 2008, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO 3.0.0