Need better structure of my code

Question

this is my front controller

$pages = array("matches", "boards", "search", "articles", "interviews", "userlist", "teams", "servers", "awards", "gallery", "qids");

if (!$_SERVER['QUERY_STRING']) include('home_en.php');
elseif (isset($_GET['matchid'])) include('matchid.php');
elseif (isset($_GET['boardid'])) include('boardid.php');
elseif (isset($_GET['articleid'])) include('articleid.php');
elseif (isset($_GET['interviewid'])) include('interviewid.php');
elseif (isset($_GET['userid'])) include('profi.php');
elseif (isset($_GET['teamid'])) include('teamid.php');
elseif (isset($_GET['serverid'])) include('serverid.php');
elseif (isset($_GET['awardid'])) include('awardid.php');
elseif (isset($_GET['galleryid'])) include('galleryid.php');
elseif (isset($_GET['threadid'])) include('threadid.php');
elseif (isset($_GET['blogid'])) include('blogid.php');
..

elseif (in_array($_GET['content'], $pages)) include($_GET['content']);

else echo "File not found =(";

could i somehow add the identifiers to the array too? but i want the pages as index.php?matchid=9438 and for regular pages: index.php?content=matches

would really aprricate some ideas

thanks!

Answer 1

My Suggestion, From My Comment is this:

In order to check what type of id it is, you should use two $_GET parameters. One is the type (match, award, server, etc), one is the ID. That way you don't have to check for 500 different $_GET parameters, just the value of 2. Much more standardized.

Second, you want to make all of it under 1 file for the ID showing.

In the spirit of writing less code, not more, it would be relatively easy to change the SQL statement to grab the record based on if $_GET['type'] was match, award, team, etc. This is of course given that they will probably look the same. If they don't, instead of writing new code to grab each type, instead write code to display it differently

All Variables in this code much be validated/sanatized beforehand.

// First Get the Type
$type = $_GET['type'];
// Then the ID
$id = $_GET['id'];

// SANITIZE YOUR DATA. Replace this with your sanitization.
die("SANITIZE YOUR DATA HERE");

// Get Data Here
$sql = "SELECT * FROM table WHERE type=".$type." AND id=".$id;
$data = mysql_query($sql);

// Next, Include a template based on the data.

// Global the variable so it can be used in the file
Global $data;

include($type."-template.php");

Answer 2

It might help to go ahead and use a framework such as Zend:

http://framework.zend.com/

Answer 3

I agree with Tom -- you should look into using a framework such as Zend, Cake, Symfony, Kohana, CodeIgniter, ez-Components, or Seagull. The advantage of using a framework is that they have already solved a lot of issues for you, including: 1) How to structure your code 2) How to interpret pretty urls (i.e. /x/1/y/2 instead of ?x=1&y=2) 3) Where to put certain types of code (html, php, configs, etc) 4) How to fix something you can't figure out (because these frameworks have communities) and much much more...

That being said, maybe you don't want all the overhead of using a framework (it does require you to learn a lot). In that case, I recommend Rasmus Lerdorf's "No Framework PHP Framework". Rasmus is the creator of PHP, so you know he knows his stuff.

Lastly, to answer your actual question, here's how I would do it:

could i somehow add the identifiers to the array too? i want the pages as index.php?matchid=9438 and for regular pages: index.php?content=matches

Sure, but yes, as Chacha102 said, you will need 2 parameters: $area (page) and $id. Example: index.php?area=articles&id=2345

Then you can re-organize & simplify your 'front controller' this way: /index.php /areas/articles.php /areas/boards.php etc. Instead of naming the templates articleid.php, just call it articles.php -- this way your area name also tells you which template to use.

$valid_areas = array("matches", "boards", "search", "articles", 
                     "interviews", "userlist", "teams", "servers", 
                     "awards", "gallery", "qids");

$area = strtolower(trim($_REQUEST['area'])); //if you are not posting any forms, use $_GET instead
$id   = (int)$_REQUEST['id']; //if you are not posting any forms, use $_GET instead

if(!$id)
{
   include('home_en.php');
}

if(!in_array($area), $valid_areas))
{
   echo 'Sorry, the area you have requested does not exist: '.$area; 
   exit();
}
else
{
   $template = '/templates/'.$area.'.php';

   if(!file_exists($template))
   {
      echo 'Sorry, the file you have requested does not exist: '.$area.' '.$id);
   }
   else
   {
      include($template);
   }
}

Answer 4

You could do this:

<?php
    $controllerDefault = 'home';

    function sanitize($str)
    {
     return str_replace(array('.', '/', '\\'), '', $str);
    }
    //Prevent of Remote File Inclusion
    $controller = sanitize($_GET['controller']);
    $id = intval($_GET['id']);

    if (empty($controller))
    {
     $controller = $controllerDefault;
    }

    if (!empty($id))
    {
     $controller .= 'id';
    }

    $controllerFile = $controller . '.php';

    if (!file_exists($controllerFile) 
           || $controller == 'index') //for not recursive index.php include :)
    {
     exit('Controller "'.$controllerFile.'" not exists');
    }
    include($controllerFile);

?>

Using this code you can use your application like:

http://yoursite.com/index.php //include('home.php')
http://yoursite.com/index.php?id=285230 //include('homeid.php')
http://yoursite.com/index.php?controller=matches //include('matches.php')
http://yoursite.com/index.php?controller=matches&id=28410 //include('matchesid.php')
http://yoursite.com/index.php?controller=notexists //ERROR! Controller "notexists" not exists
http://yoursite.com/index.php?controller=../../etc/passwd //ERROR! Controller "etcpasswd" not exists

I hope you like it

PD: the code is not tested, but I hope you catch my idea