2
\$\begingroup\$

This is a follow up question to: Admin page select function

Here, index.php does the following:

  • Checks for a login status using sessions and a cookie
  • If logged in, allow the use of the ?page= query
  • Check that the value of ?page= is in the $pagearray and that the file exists
  • If the user is not logged in, include login.php

index.php

define('WCX', TRUE);
require_once('config.php');

$pagearray = array('dashboard', 'article', 'video', 'app', 'link', 'articlelist', 'addarticle', 'videolist', 'addvideo', 'applist', 'addapp', 'linklist', 'addlink');

if(isset($_COOKIE['wcxadmin']) && isset($_SESSION['loggedin']) && isset($_SESSION['auth']) && $_SESSION['loggedin'] === $_SESSION['auth']) {

    if(isset($_GET['page']) && $_GET['page'] != '') {
        $page = filter_input(INPUT_GET, 'page', FILTER_SANITIZE_SPECIAL_CHARS);
    }
    else {
        $page = 'dashboard';    
    }

    include_once(INCLUDES.'header.php');

    if(in_array($page, $pagearray) && file_exists('includes/'.$page.'.php')) {
        include_once(INCLUDES.$page.'.php');
    }
    else {
        include_once(INCLUDES.'404.php');   
    }

    include_once(INCLUDES.'footer.php');
}
else {
    include_once('login.php');  
}
\$\endgroup\$
  • \$\begingroup\$ Honestly, looks ok to me. \$\endgroup\$ – jsanc623 Jul 18 '14 at 15:43
2
\$\begingroup\$

This looks like a "Router" to me. If you haven't already studied URL rewriting, almost every PHP framework implements it, and I think you should take a look at what's been made.

I haven't ever used it, but it looks good: Aura for PHP's Router.

I have used this one, and it's framework-dependent, but simply looking at how it's used may inspire your project: Laravel's Routing capabilities.

Of course, you may find none that suit your needs or your liking, but imitating (or forking) a pre-existing router is always an option.


Now, your existing code!

  • I'm lost on your first line. What is this global you're defining? To anyone reading your code, it has no meaning and basically starts us off on the wrong foot. Why the need for the global anyways?
  • $pagearray isn't a very good name. Hungarian notation is notorious and should be avoided.
  • I call this Spring Cleaning:

    isset($_COOKIE['wcxadmin']) && isset($_SESSION['loggedin']) && isset($_SESSION['auth']) && $_SESSION['loggedin'] === $_SESSION['auth']
    

    (magical transformation into)

    isset($_COOKIE['wcxadmin'], $_SESSION['loggedin'], $_SESSION['auth']) && $_SESSION['loggedin'] === $_SESSION['auth']
    
  • $_GET['page'] != '' should really be !empty($_GET['page']). It's easier to read.

  • It wouldn't hurt to make the third argument true in in_array($page, $pagearray). Makes for a strict type comparison in addition.
  • What's the difference between 'includes/' and INCLUDES? Shouldn't they be the same? (Make the string literal a global then)
  • INCLUDES isn't a great name, and I'd avoid the global. Notice the other URL rewriters don't use globals...
  • I see a lot of include repetition. See about refactoring this to reduce the repetition.

Overall, nice, simple, mostly concise! Looking forward to a follow-up :)

|improve this answer|||||
\$\endgroup\$
  • \$\begingroup\$ Its definitely not meant to be a re-write engine but i can see why you think that, im trying to avoid frameworks for now. define(WCX, TRUE) - in other files if WCX isnt set you cant access the file directly. $pagearray - Its the array of pages available.. I love that Magical Transformation. Yes i agree on !empty! setting TRUE in_array - I didn't even know that was a thing, thanks! I have all the directorys defined in a config.php but you have made me see it is not needed! Include is the folder with the files to include, i think its appropriate. I'll Refactor and be back! Thanks!!! \$\endgroup\$ – CodeX Jul 19 '14 at 18:31
  • \$\begingroup\$ I can see how you wouldn't intend a router! Glad I could help \$\endgroup\$ – Alex L Jul 19 '14 at 18:37

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.