Optimising a PHP If/Else statement

Question

I'm attempting to optimise the following PHP If/Else statement. Could I rewrite the code to make use to case and switch, or should I leave it as it is, or what?

Code:

if(empty($_GET['id'])){
    include('pages/home.php');
}elseif ($_GET['id'] === '13') {
    include('pages/servicestatus.php');
}elseif(!empty($_GET['id'])){
    $rawdata = fetch_article($db->real_escape_string($_GET['id']));
    if(!$rawdata){
        $title = "";
        $meta['keywords'] = "";
        $meta['description'] = "";
    }else{
        $title = stripslashes($rawdata['title']);
        $meta['keywords'] = stripslashes($rawdata['htmlkeywords']);
        $meta['description'] = stripslashes($rawdata['htmldesc']);
        $subs = stripslashes($rawdata['subs']);
        $pagecontent = "<article>" . stripslashes($rawdata['content']) . "</article>";
    }
    include("includes/header.php");
    echo $pagecontent;
    if(!$rawdata){
        error_404();
    }
}

Thanks

Dalton Conley · Accepted Answer · 2010-12-01 20:10:49Z

I hate switch statements, but its personal preference to be honest. As far as further optimization i'd suggest taking a look at some form of assembly language. It will give you some general ideas on how to make conditional statements more efficient. That is, it will give you a different out look on things.

if(!empty($_GET['id'])) 
    {

    if($_GET['id'] == '13')
    {
        include('pages/servicestatus.php');
    }
    else
    {
        $rawdata = fetch_article($db->real_escape_string($_GET['id']));

        if (!$rawdata) {

            $title = "";
            $meta['keywords'] = "";
            $meta['description'] = "";
        } else {

            $title = stripslashes($rawdata['title']);
            $meta['keywords'] = stripslashes($rawdata['htmlkeywords']);
            $meta['description'] = stripslashes($rawdata['htmldesc']);
            $subs = stripslashes($rawdata['subs']);
            $pagecontent = "<article>" . stripslashes($rawdata['content']) . "</article>";
        }

        include("includes/header.php");
        echo $pagecontent;
        if (!$rawdata) {

            error_404();
        }
    }
} 
else 
{
    include('pages/home.php');
}

Banjer · Accepted Answer · 2010-12-01 20:07:36Z

2

switch would be appropriate if you had several discrete values for $_GET['id'] that you were checking for.

One suggestion I can make for the sake of readability is that

} elseif (!empty($_GET['id'])) {

only needs to be

} else {

answered Dec 1, 2010 at 20:07

Banjer

8,3406 gold badges51 silver badges68 bronze badges

Comments

Breezer · Accepted Answer · 2010-12-01 20:12:30Z

2

Well i don't think it's necessary to switch to a swith but you could change

} elseif (!empty($_GET['id'])) {

to just

}else{

edited Dec 1, 2010 at 20:12

answered Dec 1, 2010 at 20:07

Breezer

10.5k6 gold badges32 silver badges51 bronze badges

Comments

Jonah · Accepted Answer · 2010-12-01 20:12:44Z

2

You may want to look into breaking up your code into a MVC form; that would make it much easier to maintain your code. At least put the last clause into another file, probably called default.php and include it. Also, you might create an array of id => file key/value sets, lookup the id, and include the file.

if (isset($_GET['id'])) {
    $pages = array(
        0 => 'home.php',
        13 => 'servicestatus.php'
    );
    if (isset($pages[$_GET['id']])) {
        include('pages/' . $pages[$_GET['id']]);
    } else {
        include('pages/default.php');
    }
}

edited Dec 1, 2010 at 20:12

answered Dec 1, 2010 at 20:06

Jonah

10.1k5 gold badges49 silver badges80 bronze badges

1 Comment

bear Over a year ago

As much as I would like to, the code is part of a content management system, and the majority of content is dynamic. :(

ajreal · Accepted Answer · 2010-12-01 20:08:41Z

1

Yes, switch is evaluate once, is efficient than if elseif,
and is easier to maintain with this given structure

switch ($_GET['id'])
{
  case 13: ... break;
  case 0 : ... break;
  default: ... break;
}

answered Dec 1, 2010 at 20:08

ajreal

47.4k11 gold badges97 silver badges119 bronze badges

Comments

KingCrunch · Accepted Answer · 2010-12-01 20:08:44Z

1

I dont know, if you should, or should not, but here I wouldnt. The main reason is, that there is at least one statement, you can omit, and then, you will have just a if-elseif-else-Statement

if (empty($_GET['id'])) { /* code */ }
elseif ($_GET['id'] === '13') { /* code */ }
elseif (!empty($_GET['id'])) { /* code* }

is the same as

if (empty($_GET['id'])) { /* code */ }
elseif ($_GET['id'] === '13') { /* code */ }
else { /* code* }

In the block after that, the statement if(!$rawdata) is also duplicated.

answered Dec 1, 2010 at 20:08

KingCrunch

133k22 gold badges158 silver badges174 bronze badges

2 Comments

Dalton Conley Over a year ago

Should or should not? You should always attempt to optimize your code. You sir are one of the main reasons for "bloatware".

KingCrunch Over a year ago

1) "premature optimization is the root of all evil", 2) micro-optimization 3) readable code is better than hackish code ;) and last but not least 4) nothing to optimize here ;) I dont say, you should never optimize, but here it really doesnt matter.

Collectives™ on Stack Overflow

Optimising a PHP If/Else statement

6 Answers 6

Comments

Comments

Comments

1 Comment

Comments

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

6 Answers 6

Comments

Comments

Comments

1 Comment

Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related