470,815 Members | 1,102 Online
Bytes | Developer Community
New Post

Home Posts Topics Members FAQ

Post your question to a community of 470,815 developers. It's quick & easy.

nested if's

I got lost on formatting the nested if/else's on the bottom of the
file right up the last else/if there seems to be a stray bracket. I'm
trying to line up the brackets so I can read it. Is it okay to split
a long line setting a variable on two lines at the equals sign? That
is so you can read it in google groups.

<?
//Send payement advices
$TPL_auction_id = $auction_id;
$user_id=$userrec["id"];
if(!$sessionVars["SELL_action"]) {
include "header.php";
}
if($FEE>0) {
if($SETTINGS["invoicing"] == 'y') {
if(!$sessionVars["SELL_action"]=="edit") {
include phpa_include("template_menu_php.html");
include phpa_include("template_sell_result_php.html");
} else {
$auction_url =
$SETTINGS["siteurl"] . "item.php?mode=1&id=".$auction_id;
if($sessionVars["SELL_action"]=="edit") {
unset($_SESSION["sessionVars"]);
unset($_SESSION["RELISTEDAUCTION"]);
if(isset($_SESSION['backtolist']))
header("Location: ".$_SESSION['backtolist']."?PAGE=
".$_SESSION['backtolist_page']);
else
header("Location: yourauctions.php?PAGE=
".$_SESSION['backtolist_page']);
exit;
}
if($sessionVars["SELL_action"]=="reopen") {
unset($_SESSION["sessionVars"]);
unset($_SESSION["RELISTEDAUCTION"]);
header("Location: yourauctions_c.php?PAGE=
".$_SESSION['backtolist_page']);
exit;
}
}
} elseif($SETTINGS["feetype"] == "prepay") {
if(!$sessionVars["SELL_action"]) {
//include "header.php";
}
include "lib/auctionsetup_payment.php";
} else {
if($sessionVars["SELL_action"]) {
include "header.php";
include phpa_include("template_menu_php.html");
}
?>

thanks,
Jun 2 '08 #1
13 1920
JRough wrote:
I got lost on formatting the nested if/else's on the bottom of the
file right up the last else/if there seems to be a stray bracket. I'm
trying to line up the brackets so I can read it. Is it okay to split
a long line setting a variable on two lines at the equals sign? That
is so you can read it in google groups.

<?
//Send payement advices
$TPL_auction_id = $auction_id;
$user_id=$userrec["id"];
if(!$sessionVars["SELL_action"]) {
include "header.php";
}
if($FEE>0) {
if($SETTINGS["invoicing"] == 'y') {
if(!$sessionVars["SELL_action"]=="edit") {
include phpa_include("template_menu_php.html");
include phpa_include("template_sell_result_php.html");
} else {
$auction_url =
$SETTINGS["siteurl"] . "item.php?mode=1&id=".$auction_id;
if($sessionVars["SELL_action"]=="edit") {
unset($_SESSION["sessionVars"]);
unset($_SESSION["RELISTEDAUCTION"]);
if(isset($_SESSION['backtolist']))
header("Location: ".$_SESSION['backtolist']."?PAGE=
".$_SESSION['backtolist_page']);
else
header("Location: yourauctions.php?PAGE=
".$_SESSION['backtolist_page']);
exit;
}
if($sessionVars["SELL_action"]=="reopen") {
unset($_SESSION["sessionVars"]);
unset($_SESSION["RELISTEDAUCTION"]);
header("Location: yourauctions_c.php?PAGE=
".$_SESSION['backtolist_page']);
exit;
}
}
} elseif($SETTINGS["feetype"] == "prepay") {
if(!$sessionVars["SELL_action"]) {
//include "header.php";
}
include "lib/auctionsetup_payment.php";
} else {
if($sessionVars["SELL_action"]) {
include "header.php";
include phpa_include("template_menu_php.html");
}
?>

thanks,
php is a not a card-image language like Forran. It is a free-form,
sentence language where each sentence ends in a semicolon. Thus, you
can have a sentence over more than one line.
Jun 2 '08 #2

<sheldonlgwrote in message
news:FK******************************@giganews.com ...
JRough wrote:
>I got lost on formatting the nested if/else's on the bottom of the
file right up the last else/if there seems to be a stray bracket. I'm
trying to line up the brackets so I can read it. Is it okay to split
a long line setting a variable on two lines at the equals sign? That
is so you can read it in google groups.

<?
//Send payement advices
$TPL_auction_id = $auction_id;
$user_id=$userrec["id"];
if(!$sessionVars["SELL_action"]) {
include "header.php";
}
if($FEE>0) {
if($SETTINGS["invoicing"] == 'y') {
if(!$sessionVars["SELL_action"]=="edit") {
include phpa_include("template_menu_php.html");
include phpa_include("template_sell_result_php.html");
} else {
$auction_url =
$SETTINGS["siteurl"] . "item.php?mode=1&id=".$auction_id;
if($sessionVars["SELL_action"]=="edit") {
unset($_SESSION["sessionVars"]);
unset($_SESSION["RELISTEDAUCTION"]);
if(isset($_SESSION['backtolist']))
header("Location: ".$_SESSION['backtolist']."?PAGE=
".$_SESSION['backtolist_page']);
else
header("Location: yourauctions.php?PAGE=
".$_SESSION['backtolist_page']);
exit;
}
if($sessionVars["SELL_action"]=="reopen") {
unset($_SESSION["sessionVars"]);
unset($_SESSION["RELISTEDAUCTION"]);
header("Location: yourauctions_c.php?PAGE=
".$_SESSION['backtolist_page']);
exit;
}
}
} elseif($SETTINGS["feetype"] == "prepay") {
if(!$sessionVars["SELL_action"]) {
//include "header.php";
}
include "lib/auctionsetup_payment.php";
} else {
if($sessionVars["SELL_action"]) {
include "header.php";
include phpa_include("template_menu_php.html");
}
?>

thanks,

php is a not a card-image language like Forran. It is a free-form,
sentence language where each sentence ends in a semicolon. Thus, you can
have a sentence over more than one line.
hey shelly...ur back!

just to add, always opt for a single IF. look at your else/if...very nasty
to read and maintain. just do an IF by itself (and FIRST) if the inner body
has an 'exit'. then see what the others have in common. isolate the
repetative calls with variables and 'actions'...which are what is in common.
once you've done that, actually take the action outside of ANY if...b/c this
code is ALWAYS going to do something...you're just using IF's to nest it.

again, not clean or manageable.
Jun 2 '08 #3
On May 13, 2:24 pm, JRough <jlro...@yahoo.comwrote:
I got lost on formatting the nested if/else's on the bottom of the
file right up the last else/if there seems to be a stray bracket. I'm
trying to line up the brackets so I can read it. Is it okay to split
a long line setting a variable on two lines at the equals sign? That
is so you can read it in google groups.

<?
//Send payement advices
$TPL_auction_id = $auction_id;
$user_id=$userrec["id"];
if(!$sessionVars["SELL_action"]) {
include "header.php";}

if($FEE>0) {
if($SETTINGS["invoicing"] == 'y') {
if(!$sessionVars["SELL_action"]=="edit") {
include phpa_include("template_menu_php.html");
include phpa_include("template_sell_result_php.html");
} else {
$auction_url =
$SETTINGS["siteurl"] . "item.php?mode=1&id=".$auction_id;
if($sessionVars["SELL_action"]=="edit") {
unset($_SESSION["sessionVars"]);
unset($_SESSION["RELISTEDAUCTION"]);
if(isset($_SESSION['backtolist']))
header("Location: ".$_SESSION['backtolist']."?PAGE=
".$_SESSION['backtolist_page']);
else
header("Location: yourauctions.php?PAGE=
".$_SESSION['backtolist_page']);
exit;
}
if($sessionVars["SELL_action"]=="reopen") {
unset($_SESSION["sessionVars"]);
unset($_SESSION["RELISTEDAUCTION"]);
header("Location: yourauctions_c.php?PAGE=
".$_SESSION['backtolist_page']);
exit;
}}
} elseif($SETTINGS["feetype"] == "prepay") {

if(!$sessionVars["SELL_action"]) {
//include "header.php";}

include "lib/auctionsetup_payment.php";} else {

if($sessionVars["SELL_action"]) {
include "header.php";
include phpa_include("template_menu_php.html");}

?>
The first thing I think of when something with this much logic comes
up is a switch statement. Of course sometimes it is not suited at all
or only for some of the logic. However, when you can use switch, the
code is much more orderly and easy to modify. You may have already
considered switch and found problems in using it, so excuse me for
bringing up the subject if that is the case. I do not have time just
now to consider the details of your code and test them. The php switch
is nearly the same as a Javascript switch in the formal code structure.
Jun 2 '08 #4
Barry wrote:
<sheldonlgwrote in message
news:FK******************************@giganews.com ...
>JRough wrote:
>>I got lost on formatting the nested if/else's on the bottom of the
file right up the last else/if there seems to be a stray bracket. I'm
trying to line up the brackets so I can read it. Is it okay to split
a long line setting a variable on two lines at the equals sign? That
is so you can read it in google groups.

<?
//Send payement advices
$TPL_auction_id = $auction_id;
$user_id=$userrec["id"];
if(!$sessionVars["SELL_action"]) {
include "header.php";
}
if($FEE>0) {
if($SETTINGS["invoicing"] == 'y') {
if(!$sessionVars["SELL_action"]=="edit") {
include phpa_include("template_menu_php.html");
include phpa_include("template_sell_result_php.html");
} else {
$auction_url =
$SETTINGS["siteurl"] . "item.php?mode=1&id=".$auction_id;
if($sessionVars["SELL_action"]=="edit") {
unset($_SESSION["sessionVars"]);
unset($_SESSION["RELISTEDAUCTION"]);
if(isset($_SESSION['backtolist']))
header("Location: ".$_SESSION['backtolist']."?PAGE=
".$_SESSION['backtolist_page']);
else
header("Location: yourauctions.php?PAGE=
".$_SESSION['backtolist_page']);
exit;
}
if($sessionVars["SELL_action"]=="reopen") {
unset($_SESSION["sessionVars"]);
unset($_SESSION["RELISTEDAUCTION"]);
header("Location: yourauctions_c.php?PAGE=
".$_SESSION['backtolist_page']);
exit;
}
}
} elseif($SETTINGS["feetype"] == "prepay") {
if(!$sessionVars["SELL_action"]) {
//include "header.php";
}
include "lib/auctionsetup_payment.php";
} else {
if($sessionVars["SELL_action"]) {
include "header.php";
include phpa_include("template_menu_php.html");
}
?>

thanks,
php is a not a card-image language like Forran. It is a free-form,
sentence language where each sentence ends in a semicolon. Thus, you can
have a sentence over more than one line.

hey shelly...ur back!
I was never really away. I just (a) was **extremely** busy and (2) had
some unexpected medical issues (I'm alright). I looked in fairly
regularly, but generally when I saw one to comment on, it had already
been answered quite well.
>
just to add, always opt for a single IF. look at your else/if...very nasty
to read and maintain. just do an IF by itself (and FIRST) if the inner body
has an 'exit'. then see what the others have in common. isolate the
repetative calls with variables and 'actions'...which are what is in common.
once you've done that, actually take the action outside of ANY if...b/c this
code is ALWAYS going to do something...you're just using IF's to nest it.

again, not clean or manageable.
Switches are fine if you have multiple branching on a single test. He
does have some of that with his 'if($sessionVars["SELL_action"]'. In
any event, he should go to strict two character tabs so that he doesn't
get lost at the end of the line and have line wrap problems.
Jun 2 '08 #5
some vertical whitespace and braces around all if's and else's
not sure yet how posting will mangle it.

<?

//Send payement advices
$TPL_auction_id= $auction_id;
$user_id= $userrec["id"];

if(!$sessionVars["SELL_action"]) {
include "header.php";
}

if($FEE 0) {
if($SETTINGS["invoicing"] == 'y') {
if(!$sessionVars["SELL_action"] == "edit") {
include phpa_include("template_menu_php.html");
include phpa_include("template_sell_result_php.html");
} else {
$auction_url=
$SETTINGS["siteurl"] . "item.php?mode=1&id=".$auction_id;

if($sessionVars["SELL_action"] == "edit") {
unset($_SESSION["sessionVars"]);
unset($_SESSION["RELISTEDAUCTION"]);

if(isset($_SESSION['backtolist'])){
header("Location: ".$_SESSION['backtolist']."?PAGE=
".$_SESSION['backtolist_page']);
}
else{
header("Location: yourauctions.php?PAGE=
".$_SESSION['backtolist_page']);
}
exit;
}

if($sessionVars["SELL_action"]=="reopen") {
unset($_SESSION["sessionVars"]);
unset($_SESSION["RELISTEDAUCTION"]);
header("Location: yourauctions_c.php?PAGE=
".$_SESSION['backtolist_page']);
exit;
}
}
} elseif($SETTINGS["feetype"] == "prepay") {
if(!$sessionVars["SELL_action"]) {
//include "header.php";
}
include "lib/auctionsetup_payment.php";
} else {

if($sessionVars["SELL_action"]) {
include "header.php";
include phpa_include("template_menu_php.html");
}

// } missing close else brace
?>
"JRough" <jl*****@yahoo.comwrote in message
news:44**********************************@u12g2000 prd.googlegroups.com...
>I got lost on formatting the nested if/else's on the bottom of the
file right up the last else/if there seems to be a stray bracket. I'm
trying to line up the brackets so I can read it. Is it okay to split
a long line setting a variable on two lines at the equals sign? That
is so you can read it in google groups.

<?
//Send payement advices
$TPL_auction_id = $auction_id;
$user_id=$userrec["id"];
if(!$sessionVars["SELL_action"]) {
include "header.php";
}
if($FEE>0) {
if($SETTINGS["invoicing"] == 'y') {
if(!$sessionVars["SELL_action"]=="edit") {
include phpa_include("template_menu_php.html");
include phpa_include("template_sell_result_php.html");
} else {
$auction_url =
$SETTINGS["siteurl"] . "item.php?mode=1&id=".$auction_id;
if($sessionVars["SELL_action"]=="edit") {
unset($_SESSION["sessionVars"]);
unset($_SESSION["RELISTEDAUCTION"]);
if(isset($_SESSION['backtolist']))
header("Location: ".$_SESSION['backtolist']."?PAGE=
".$_SESSION['backtolist_page']);
else
header("Location: yourauctions.php?PAGE=
".$_SESSION['backtolist_page']);
exit;
}
if($sessionVars["SELL_action"]=="reopen") {
unset($_SESSION["sessionVars"]);
unset($_SESSION["RELISTEDAUCTION"]);
header("Location: yourauctions_c.php?PAGE=
".$_SESSION['backtolist_page']);
exit;
}
}
} elseif($SETTINGS["feetype"] == "prepay") {
if(!$sessionVars["SELL_action"]) {
//include "header.php";
}
include "lib/auctionsetup_payment.php";
} else {
if($sessionVars["SELL_action"]) {
include "header.php";
include phpa_include("template_menu_php.html");
}
?>

thanks,

Jun 2 '08 #6
JRough wrote:
I got lost on formatting the nested if/else's on the bottom of the
file right up the last else/if there seems to be a stray bracket. I'm
trying to line up the brackets so I can read it. Is it okay to split
a long line setting a variable on two lines at the equals sign? That
is so you can read it in google groups.
<snip code>
>
thanks,
Joe beat me to it :-)

In addition, get yourself an editor with syntax highlighting. The main
one I use is Crimson Editor (http://www.crimsoneditor.com/) - free, fast
and good.

--
==================
Remove the "x" from my email address
Jerry Stuckle
JDS Computer Training Corp.
js*******@attglobal.net
==================

Jun 2 '08 #7

<sheldonlgwrote in message
news:ss******************************@giganews.com ...
Barry wrote:
><sheldonlgwrote in message
news:FK******************************@giganews.co m...
>>JRough wrote:
I got lost on formatting the nested if/else's on the bottom of the
file right up the last else/if there seems to be a stray bracket. I'm
trying to line up the brackets so I can read it. Is it okay to split
a long line setting a variable on two lines at the equals sign? That
is so you can read it in google groups.

<?
//Send payement advices
$TPL_auction_id = $auction_id;
$user_id=$userrec["id"];
if(!$sessionVars["SELL_action"]) {
include "header.php";
}
if($FEE>0) {
if($SETTINGS["invoicing"] == 'y') {
if(!$sessionVars["SELL_action"]=="edit") {
include phpa_include("template_menu_php.html");
include phpa_include("template_sell_result_php.html");
} else {
$auction_url =
$SETTINGS["siteurl"] . "item.php?mode=1&id=".$auction_id;
if($sessionVars["SELL_action"]=="edit") {
unset($_SESSION["sessionVars"]);
unset($_SESSION["RELISTEDAUCTION"]);
if(isset($_SESSION['backtolist']))
header("Location: ".$_SESSION['backtolist']."?PAGE=
".$_SESSION['backtolist_page']);
else
header("Location: yourauctions.php?PAGE=
".$_SESSION['backtolist_page']);
exit;
}
if($sessionVars["SELL_action"]=="reopen") {
unset($_SESSION["sessionVars"]);
unset($_SESSION["RELISTEDAUCTION"]);
header("Location: yourauctions_c.php?PAGE=
".$_SESSION['backtolist_page']);
exit;
}
}
} elseif($SETTINGS["feetype"] == "prepay") {
if(!$sessionVars["SELL_action"]) {
//include "header.php";
}
include "lib/auctionsetup_payment.php";
} else {
if($sessionVars["SELL_action"]) {
include "header.php";
include phpa_include("template_menu_php.html");
}
?>

thanks,
php is a not a card-image language like Forran. It is a free-form,
sentence language where each sentence ends in a semicolon. Thus, you
can have a sentence over more than one line.

hey shelly...ur back!

I was never really away. I just (a) was **extremely** busy and (2) had
some unexpected medical issues (I'm alright). I looked in fairly
regularly, but generally when I saw one to comment on, it had already been
answered quite well.
>>
just to add, always opt for a single IF. look at your else/if...very
nasty to read and maintain. just do an IF by itself (and FIRST) if the
inner body has an 'exit'. then see what the others have in common.
isolate the repetative calls with variables and 'actions'...which are
what is in common. once you've done that, actually take the action
outside of ANY if...b/c this code is ALWAYS going to do
something...you're just using IF's to nest it.

again, not clean or manageable.

Switches are fine if you have multiple branching on a single test. He
does have some of that with his 'if($sessionVars["SELL_action"]'. In any
event, he should go to strict two character tabs so that he doesn't get
lost at the end of the line and have line wrap problems.
right...i meant reducing the if/else's themselves...like:

if ($someVar)
{
// some action
exit;
}
// no 'else' nesting required because logically it is not needed.

just a nuance though. but yes, sell_action is redundantly analyzed and done
so, far apart in the nesting which could get confusing. a little more than
6, 1, 0.5, the other...but not much more based on the snippet. i agree with
you that the first, best thing to do is simply provide better formatting.

glad you're alright! i'll email you later...it's been a while.
Jun 2 '08 #8
On 13 May, 23:43, cwdjrxyz <spamtr...@cwdjr.infowrote:
On May 13, 2:24 pm, JRough <jlro...@yahoo.comwrote:
I got lost on formatting the nested if/else's on the bottom of the
file right up the last else/if there seems to be a stray bracket. I'm
trying to line up the brackets so I can read it. Is it okay to split
a long line setting a variable on two lines at the equals sign? That
is so you can read it in google groups.
<?
//Send payement advices
$TPL_auction_id = $auction_id;
$user_id=$userrec["id"];
if(!$sessionVars["SELL_action"]) {
include "header.php";}
if($FEE>0) {
if($SETTINGS["invoicing"] == 'y') {
if(!$sessionVars["SELL_action"]=="edit") {
include phpa_include("template_menu_php.html");
include phpa_include("template_sell_result_php.html");
} else {
$auction_url =
$SETTINGS["siteurl"] . "item.php?mode=1&id=".$auction_id;
if($sessionVars["SELL_action"]=="edit") {
unset($_SESSION["sessionVars"]);
unset($_SESSION["RELISTEDAUCTION"]);
if(isset($_SESSION['backtolist']))
header("Location: ".$_SESSION['backtolist']."?PAGE=
".$_SESSION['backtolist_page']);
else
header("Location: yourauctions.php?PAGE=
".$_SESSION['backtolist_page']);
exit;
}
if($sessionVars["SELL_action"]=="reopen") {
unset($_SESSION["sessionVars"]);
unset($_SESSION["RELISTEDAUCTION"]);
header("Location: yourauctions_c.php?PAGE=
".$_SESSION['backtolist_page']);
exit;
}}
} elseif($SETTINGS["feetype"] == "prepay") {
if(!$sessionVars["SELL_action"]) {
//include "header.php";}
include "lib/auctionsetup_payment.php";} else {
if($sessionVars["SELL_action"]) {
include "header.php";
include phpa_include("template_menu_php.html");}
?>

The first thing I think of when something with this much logic comes
up is a switch statement. Of course sometimes it is not suited at all
or only for some of the logic. However, when you can use switch, the
code is much more orderly and easy to modify. You may have already
considered switch and found problems in using it, so excuse me for
bringing up the subject if that is the case. I do not have time just
now to consider the details of your code and test them. The php switch
is nearly the same as a Javascript switch in the formal code structure.
A PHP switch statement only really works on the value(s) of a single
expression - the code above relies on multiple experssions. I'd agree
that the code above is rather messy and difficult to maintain.
Certainly end brackets of nested blocks should be commented to
reference the start point. It already looks as if it needs fixed:

if(!$sessionVars["SELL_action"]=="edit") {
....
} else {
...
if($sessionVars["SELL_action"]=="edit") {
There's other ways to make the code clearer though:
You could break it down into seperate functions.
The indentiy of include files is alerady encapsulated in a function
(presumably for path rewriting) you could push the logic for determing
which include files to use further down the call tree.
You could bundle the different scenarios under single level if ...
elseif ...else statements
e.g.

//Send payement advices
$TPL_auction_id = $auction_id;
$user_id=$userrec["id"];

if ((!$sessionVars["SELL_action"]) || ($FEE<=0) {
include "header.php";
}
if (($FEE>0) && (($SETTINGS["invoicing"] == 'y') && (!
$sessionVars["SELL_action"]=="edit")) {
include phpa_include("template_menu_php.html");
include phpa_include("template_sell_result_php.html");
}
if (($FEE>0) && (($SETTINGS["invoicing"] == 'y') && (!
$sessionVars["SELL_action"]!="edit")) {
$auction_url = $SETTINGS["siteurl"] . "item.php?mode=1&id=".
$auction_id;
}
....

Another problem with the code is that it seems to rely on inline
(main) code within include functions - which can introduce lots of
unexpected side-effects and create vulnerabilities. As a general rule
I don't have any inline code other than defines in include files. If
you use OO then you can use the autlolader to selectively load
libraries.

Another thing I note is that you use include throughout where require
might_once might be more appropriate.

Sorry OP, but the length of your lines is the least of your worries.
You should start by reading up on Karnaugh maps and boolean algebra.

C.
Jun 2 '08 #9

"C. (http://symcbean.blogspot.com/)" <co************@gmail.comwrote in
message
news:86**********************************@r66g2000 hsg.googlegroups.com...
On 13 May, 23:43, cwdjrxyz <spamtr...@cwdjr.infowrote:
>On May 13, 2:24 pm, JRough <jlro...@yahoo.comwrote:
I got lost on formatting the nested if/else's on the bottom of the
file right up the last else/if there seems to be a stray bracket. I'm
trying to line up the brackets so I can read it. Is it okay to split
a long line setting a variable on two lines at the equals sign? That
is so you can read it in google groups.
<?
//Send payement advices
$TPL_auction_id = $auction_id;
$user_id=$userrec["id"];
if(!$sessionVars["SELL_action"]) {
include "header.php";}
if($FEE>0) {
if($SETTINGS["invoicing"] == 'y') {
if(!$sessionVars["SELL_action"]=="edit") {
include phpa_include("template_menu_php.html");
include
phpa_include("template_sell_result_php.html");
} else {
$auction_url =
$SETTINGS["siteurl"] .
"item.php?mode=1&id=".$auction_id;
if($sessionVars["SELL_action"]=="edit")
{

unset($_SESSION["sessionVars"]);

unset($_SESSION["RELISTEDAUCTION"]);

if(isset($_SESSION['backtolist']))

header("Location: ".$_SESSION['backtolist']."?PAGE=

".$_SESSION['backtolist_page']);
else

header("Location: yourauctions.php?PAGE=

".$_SESSION['backtolist_page']);
exit;
}
if($sessionVars["SELL_action"]=="reopen") {
unset($_SESSION["sessionVars"]);
unset($_SESSION["RELISTEDAUCTION"]);
header("Location: yourauctions_c.php?PAGE=
".$_SESSION['backtolist_page']);
exit;
}}
} elseif($SETTINGS["feetype"] == "prepay") {
if(!$sessionVars["SELL_action"]) {
//include "header.php";}
include "lib/auctionsetup_payment.php";} else {
if($sessionVars["SELL_action"]) {
include "header.php";
include phpa_include("template_menu_php.html");}
?>

The first thing I think of when something with this much logic comes
up is a switch statement. Of course sometimes it is not suited at all
or only for some of the logic. However, when you can use switch, the
code is much more orderly and easy to modify. You may have already
considered switch and found problems in using it, so excuse me for
bringing up the subject if that is the case. I do not have time just
now to consider the details of your code and test them. The php switch
is nearly the same as a Javascript switch in the formal code structure.

A PHP switch statement only really works on the value(s) of a single
expression - the code above relies on multiple experssions.
switch (true)
{
case $sessionVars['SELL_action'] == 'edit' :
}

that's a single expression value...you can have as little or as few
expressions in 'case'. php would be looking for 'true'.
I'd agree
that the code above is rather messy and difficult to maintain.
Certainly end brackets of nested blocks should be commented to
reference the start point. It already looks as if it needs fixed:

if(!$sessionVars["SELL_action"]=="edit") {
....
} else {
...
if($sessionVars["SELL_action"]=="edit") {
There's other ways to make the code clearer though:
You could break it down into seperate functions.
The indentiy of include files is alerady encapsulated in a function
(presumably for path rewriting) you could push the logic for determing
which include files to use further down the call tree.
You could bundle the different scenarios under single level if ...
elseif ...else statements
e.g.

//Send payement advices
$TPL_auction_id = $auction_id;
$user_id=$userrec["id"];

if ((!$sessionVars["SELL_action"]) || ($FEE<=0) {
include "header.php";
}
if (($FEE>0) && (($SETTINGS["invoicing"] == 'y') && (!
$sessionVars["SELL_action"]=="edit")) {
include phpa_include("template_menu_php.html");
include phpa_include("template_sell_result_php.html");
}
if (($FEE>0) && (($SETTINGS["invoicing"] == 'y') && (!
$sessionVars["SELL_action"]!="edit")) {
$auction_url = $SETTINGS["siteurl"] . "item.php?mode=1&id=".
$auction_id;
}
...
you have to be careful though, since if/elseif/else constructs are
exclusionary. not nesting the if's when needed will have less than
beneficial results.
Another problem with the code is that it seems to rely on inline
(main) code within include functions - which can introduce lots of
unexpected side-effects and create vulnerabilities. As a general rule
I don't have any inline code other than defines in include files. If
you use OO then you can use the autlolader to selectively load
libraries.

Another thing I note is that you use include throughout where require
might_once might be more appropriate.

Sorry OP, but the length of your lines is the least of your worries.
You should start by reading up on Karnaugh maps and boolean algebra.
lol. that's a thought.
Jun 2 '08 #10
On May 13, 6:53 pm, Jerry Stuckle <jstuck...@attglobal.netwrote:
JRough wrote:
I got lost on formatting the nested if/else's on the bottom of the
file right up the last else/if there seems to be a stray bracket. I'm
trying to line up the brackets so I can read it. Is it okay to split
a long line setting a variable on two lines at the equals sign? That
is so you can read it in google groups.

<snip code>
thanks,

Joe beat me to it :-)

In addition, get yourself an editor with syntax highlighting. The main
one I use is Crimson Editor (http://www.crimsoneditor.com/) - free, fast
and good.

--
==================
Remove the "x" from my email address
Jerry Stuckle
JDS Computer Training Corp.
jstuck...@attglobal.net
==================
tnx, I was wondering if there was a case statement, I will look for
the switch instead, I'm coming from programming in VBA.
Jun 2 '08 #11
On May 14, 9:03 am, "Barry" <no....@example.comwrote:
"C. (http://symcbean.blogspot.com/)" <colin.mckin...@gmail.comwrote in
messagenews:86**********************************@r 66g2000hsg.googlegroups.com...
On 13 May, 23:43, cwdjrxyz <spamtr...@cwdjr.infowrote:
On May 13, 2:24 pm, JRough <jlro...@yahoo.comwrote:
I got lost on formatting the nested if/else's on the bottom of the
file right up the last else/if there seems to be a stray bracket. I'm
trying to line up the brackets so I can read it. Is it okay to split
a long line setting a variable on two lines at the equals sign? That
is so you can read it in google groups.
<?
//Send payement advices
$TPL_auction_id = $auction_id;
$user_id=$userrec["id"];
if(!$sessionVars["SELL_action"]) {
include "header.php";}
if($FEE>0) {
if($SETTINGS["invoicing"] == 'y') {
if(!$sessionVars["SELL_action"]=="edit") {
include phpa_include("template_menu_php.html");
include
phpa_include("template_sell_result_php.html");
} else {
$auction_url =
$SETTINGS["siteurl"] .
"item.php?mode=1&id=".$auction_id;
if($sessionVars["SELL_action"]=="edit")
{
unset($_SESSION["sessionVars"]);
unset($_SESSION["RELISTEDAUCTION"]);
if(isset($_SESSION['backtolist']))
header("Location: ".$_SESSION['backtolist']."?PAGE=
".$_SESSION['backtolist_page']);
else
header("Location: yourauctions.php?PAGE=
".$_SESSION['backtolist_page']);
exit;
}
if($sessionVars["SELL_action"]=="reopen") {
unset($_SESSION["sessionVars"]);
unset($_SESSION["RELISTEDAUCTION"]);
header("Location: yourauctions_c.php?PAGE=
".$_SESSION['backtolist_page']);
exit;
}}
} elseif($SETTINGS["feetype"] == "prepay") {
if(!$sessionVars["SELL_action"]) {
//include "header.php";}
include "lib/auctionsetup_payment.php";} else {
if($sessionVars["SELL_action"]) {
include "header.php";
include phpa_include("template_menu_php.html");}
?>
The first thing I think of when something with this much logic comes
up is a switch statement. Of course sometimes it is not suited at all
or only for some of the logic. However, when you can use switch, the
code is much more orderly and easy to modify. You may have already
considered switch and found problems in using it, so excuse me for
bringing up the subject if that is the case. I do not have time just
now to consider the details of your code and test them. The php switch
is nearly the same as a Javascript switch in the formal code structure.
A PHP switch statement only really works on the value(s) of a single
expression - the code above relies on multiple experssions.

switch (true)
{
case $sessionVars['SELL_action'] == 'edit' :

}

that's a single expression value...you can have as little or as few
expressions in 'case'. php would be looking for 'true'.
I'd agree
that the code above is rather messy and difficult to maintain.
Certainly end brackets of nested blocks should be commented to
reference the start point. It already looks as if it needs fixed:
if(!$sessionVars["SELL_action"]=="edit") {
....
} else {
...
if($sessionVars["SELL_action"]=="edit") {
There's other ways to make the code clearer though:
You could break it down into seperate functions.
The indentiy of include files is alerady encapsulated in a function
(presumably for path rewriting) you could push the logic for determing
which include files to use further down the call tree.
You could bundle the different scenarios under single level if ...
elseif ...else statements
e.g.
//Send payement advices
$TPL_auction_id = $auction_id;
$user_id=$userrec["id"];
if ((!$sessionVars["SELL_action"]) || ($FEE<=0) {
include "header.php";
}
if (($FEE>0) && (($SETTINGS["invoicing"] == 'y') && (!
$sessionVars["SELL_action"]=="edit")) {
include phpa_include("template_menu_php.html");
include phpa_include("template_sell_result_php.html");
}
if (($FEE>0) && (($SETTINGS["invoicing"] == 'y') && (!
$sessionVars["SELL_action"]!="edit")) {
$auction_url = $SETTINGS["siteurl"] . "item.php?mode=1&id=".
$auction_id;
}
...

you have to be careful though, since if/elseif/else constructs are
exclusionary. not nesting the if's when needed will have less than
beneficial results.
Another problem with the code is that it seems to rely on inline
(main) code within include functions - which can introduce lots of
unexpected side-effects and create vulnerabilities. As a general rule
I don't have any inline code other than defines in include files. If
you use OO then you can use the autlolader to selectively load
libraries.
Another thing I note is that you use include throughout where require
might_once might be more appropriate.
Sorry OP, but the length of your lines is the least of your worries.
You should start by reading up on Karnaugh maps and boolean algebra.

lol. that's a thought.
thanks,
Jun 2 '08 #12
On 14 May, 17:03, "Barry" <no....@example.comwrote:
"C. (http://symcbean.blogspot.com/)" <colin.mckin...@gmail.comwrote in
messagenews:86**********************************@r 66g2000hsg.googlegroups.com...
A PHP switch statement only really works on the value(s) of a single
expression - the code above relies on multiple experssions.

switch (true)
{
case $sessionVars['SELL_action'] == 'edit' :

}

that's a single expression value...you can have as little or as few
expressions in 'case'. php would be looking for 'true'.
Cool. I should have thought of that.

Ta

C.
Jun 2 '08 #13
JRough wrote:
trying to line up the brackets so I can read it. Is it okay to split
a long line setting a variable on two lines at the equals sign? That
....
header("Location: ".$_SESSION['backtolist']."?PAGE=
".$_SESSION['backtolist_page']);
No. End of line and all these spaces now are parts of text constant
and will go direct to string. It's bad. Do break on point operator:

header("Location: ".$_SESSION['backtolist']."?PAGE="
.$_SESSION['backtolist_page']);

And use functions to reduce nesting level.
Jun 2 '08 #14

This discussion thread is closed

Replies have been disabled for this discussion.

Similar topics

6 posts views Thread by Andy Baker | last post: by
3 posts views Thread by Erik Bongers | last post: by
10 posts views Thread by nimmi_srivastav | last post: by
6 posts views Thread by B0nj | last post: by
8 posts views Thread by Robert W. | last post: by
37 posts views Thread by Tim N. van der Leeuw | last post: by
7 posts views Thread by patrick j | last post: by
3 posts views Thread by jdurancomas | last post: by
By using this site, you agree to our Privacy Policy and Terms of Use.