CODEWITHSUNDEEP
phpoptimizationcaseif-statement
bear
bear

Reputation: 11615

Optimising a PHP If/Else statement

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

Upvotes: 4

Views: 2065

Answers (6)

Jonah
Jonah

Reputation: 10091

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');
    }
}

Upvotes: 2

Breezer
Breezer

Reputation: 10490

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

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

to just 

}else{

Upvotes: 2

Dalton Conley
Dalton Conley

Reputation: 1615

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');
}

Upvotes: 3

KingCrunch
KingCrunch

Reputation: 131881

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.

Upvotes: 1

ajreal
ajreal

Reputation: 47321

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;
}

Upvotes: 1

Banjer
Banjer

Reputation: 8300

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 {

Upvotes: 2

Related Questions