0

Here is my case. I have the following script:

<?php
// ... other code ...
switch($_GET['request'])    {
        case "firstPage":
            $file = APP_ROOT. "pages/firstPage.php";
            $hndle = fopen($file,"r");
            $right_column = fread($hndle, filesize($file));
            fclose($hndle);
            break;
        case "secondPage":
            $file = APP_ROOT. "pages/secondPage.php";
            $hndle = fopen($file,"r");
            $right_column = fread($hndle, filesize($file));
            fclose($hndle);
            break;
    }
}
?>
<div id="right_column">
    <?php echo $right_column;?>
</div>
// ... other code ...

Depending on the value of the $_GET['request'] I am assigning to the variable $right_column the content of a php file. Then, I echo that variable in the last div. The firstPage.php and secondPage.php files contain mixed html and php code. I look for a solution like the 'partial' in Zend. Thanks

Ginger Opariti
  • 1,282
  • 5
  • 23
  • 41

4 Answers4

2

First of all , your code is horrible, it should be something like :

$pages = array(
   'firstPage'   => 'pages/firstPage.php',
   'secondPage'  => 'pages/secondPage.php'
);

$request = 'firstPage';
if ( array_key_exits($_GET, 'request') 
 &&  array_key_exists($pages, $_GET['request']) )
{
   $request = $_GET['request'];
}
?>
<div id="right_column">
    <?php include  APP_ROOT . $pages[ $request ]; ?>
</div>

An please read http://codeangel.org/articles/simple-php-template-engine.html , you might find this interesting. It should explain to you , how you can easily take advantage of php's native templating capabilities.

tereško
  • 58,060
  • 25
  • 98
  • 150
  • your comment, in essence in line with the others above, reminds me of the early competitions to write the most unreadable C code possible. You have your point of view but I'm skeptical to reply on your 'horrible' attribute. ;-) though ... – Ginger Opariti Nov 22 '11 at 17:14
  • @GingerOpariti , then try adding 12 different pages in your version .. and forget empty `request` parameter in url. – tereško Nov 22 '11 at 17:23
  • +1 This is definitely better than an ever expanding switch statement. – Fenton Nov 23 '11 at 10:09
1

Actually, reading and dumping your file is not a solution. You are just displaying code to your end-user.

What you need is parse the PHP file and display the result. You can do it in 2 ways:

First, intead of

 $file = APP_ROOT. "pages/firstPage.php";
 $hndle = fopen($file,"r");
 $right_column = fread($hndle, filesize($file));
 fclose($hndle);

You can do:

$right_column = include(APP_ROOT . "pages/firstPage.php");

So, your firstPage.php must RETURN the code. Something like this:

// firstPage.php
return "my html";

But you also can include it like this:

<div id="right_column">
    <?php include(APP_ROOT . "pages/firstPage.php"); ?>
</div>

Or, you can use the ob_get_contents. PHP has a nice documentation about how to use it.

I'd use include for your case.

gustavotkg
  • 4,099
  • 1
  • 19
  • 29
0

It would probably be simpler if you set a variable indicating which PHP file to include, and then rather than reading the file, just include it. For example:

<?php
// ... other code ...
switch($_GET['request'])    {
        case "firstPage":
            $file = APP_ROOT. "pages/firstPage.php";
            break;
        case "secondPage":
            $file = APP_ROOT. "pages/secondPage.php";
            break;
    }
}
?>
<div id="right_column">
    <?php include($file);?>
</div>
// ... other code ...

If your firstPage.php and secondPage.php files only had HTML, what you're doing would work. If it has PHP, then you'll need to include() it; or you could eval() it but that is again doing more work than you need.

jprofitt
  • 10,874
  • 4
  • 36
  • 46
  • Thanks. Common sense, there is a huge backlog of questions with this subject, the solution was handy but amazingly, nobody saw it, including myself! – Ginger Opariti Nov 22 '11 at 17:03
-1

It seems to me that you could program by convention rather than have an ever increasing switch statement:

<?php
// ... other code ...

$file = '';
if (isset($_GET['request'])) {
    $safeFileName = sanitizeRequestForFileName($_GET['request']);
    $file = APP_ROOT. 'pages/' . $safeFileName . '.php';
}
?>
<div id="right_column">
    <?php 
        if (file_exists($file)) {
            include($file);
        }
    ?>
</div>
// ... other code ...
Fenton
  • 241,084
  • 71
  • 387
  • 401
  • Are you sure that including a file just because it exists is the best idea ? There can be a lot of things inside `$_GET['request']`. – tereško Nov 22 '11 at 17:26
  • This is extremely insecure. What if `$_GET['request']` === `../uploads/hacker_script`? Oops. – Chris Baker Nov 22 '11 at 18:24
  • @Chris that really isn't a reason to avoid programming by convention. You would simply write a method to sanitize the input, or use a store such as tereško's answer if you want to lock down the options. I just believe it is better if you don't need to change your code each time you want to add an extra option. – Fenton Nov 23 '11 at 10:07