1
\$\begingroup\$

I'm writing a MVC app, I've put effort into writing index.php since it must be the entrance point (like main for C and Java) imho. I would like to ensure if someone who were to work on this file wouldn't be confused.

require_once 'App/config.php'; //main constants are defined here
require_once 'App/autoload.php';

if(strpos(URL,'error/'))
    goto start;
try{
    Connection::set(DBMS,HOST,PORT,DB_NAME,DB_USER,DB_PWD,$PDO_OPTIONS);
    DAO::init();

}catch(Exception $e){
    if(!PROD){ //if the app is online PROD=TRUE
        throw $e;

    }else
        header('location: '.WEBROOT.'error/503/',503);

    exit;
}

session_start();

if(!isset($_SESSION['user'])) 
    $_SESSION["user"] = new Guest;

start:
    try{
        extract($_GET);
        unset($_GET);

        if(isset($controller, $action, $params))
            Dispatcher::dispatch($controller, $action, $params);

        else if (isset($controller, $action))
            Dispatcher::dispatch($controller, $action);

        else if (isset($controller, $params))
            Dispatcher::dispatch($controller, NULL, $params);

        else if (isset($controller))
            Dispatcher::dispatch($controller);

        else
            Dispatcher::dispatch();

        echo Dispatcher::deliver(); //output the response

    }catch(Throwable $t){
        if(!PROD)
            throw $t;

        else if($t instanceof TypeError && strpos($t->getTrace()[0],'Dispatcher.php'))
            header('location: '.WEBROOT.'error/400/',400);

        else if(strpos($t->getMessage(),'not found'))
            header('location: '.WEBROOT.'error/404/',404);

        else
            header('location: '.WEBROOT.'error/503/',503);
    }
\$\endgroup\$
1
\$\begingroup\$

Goto

Honestly, I didn't even know that PHP supported labels and goto. But this is how the manual for PHPs goto starts:

xkcd comic: goto is evil

If you don't have a very good reason to use goto, just don't use it. It generally leads to bad code.

What you have is basically this:

if(strpos(URL,'error/'))
    goto start;

// init database

start:
    // init app

Or in other words:

if error:
    do not start database connection

start program

So you can rephrase your confusing goto statement like this:

if(!strpos(URL,'error/')) {
    initDatabase();
}

initApp();

function initDatabase() {
    // Connection::set(...)
}

function initApp() {
    // start session, call dispatcher, etc
}

Formatting

You should always use curly brackets, even for one-line statements. If you absolutely think that you need to omit them (you don't), be consistent. This for example isn't easy to read:

if(!PROD){ //if the app is online PROD=TRUE
    throw $e;

}else
    header('location: '.WEBROOT.'error/503/',503);

exit;

But something like this also isn't clear at first glance:

if(strpos(URL,'error/'))
    goto start;
try{

At the very least add a new paragraph. But really, curly brackets are the right thing here.

header redirect

In your case it isn't strictly necessary, but it's good practice to always die after a header redirect as a client doesn't has to follow it, so code that comes after it will be executed as well.

This may cause you problems in case you add further code later on, and expect the redirect to stop execution.

Extract

Extract is not a very good function. It's not secure to let a user have complete control over your variables, and it leads to bad code, as it's unclear what variables are defined.

Just assign the variables you need directly.

Misc

  • I wouldn't hard-code URLs such as your error URL. Ideally, URLs are customizable in some config file.
\$\endgroup\$

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.