Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Does this code contain any useless parts? Im am using it for a fast JSON API without routes. Is the class loader efficient enough? Are there any better options? Is ReflectionMethod slow if used once?

<?php

/*
 * Init stuff
 */

session_start();

define('ROOT', realpath( dirname(__FILE__) ) . '/');
set_include_path(ROOT . 'src/');

spl_autoload_extensions('.php');
spl_autoload_register();

ini_set('error_reporting', E_ALL & ~E_DEPRECATED);
ini_set('display_errors',  1); //To change in prod to 0 <<<<<<<<<<<<<<<<<<<<<<<<<
ini_set('log_errors',      1);
ini_set('error_log',       ROOT . 'tmp/logs/application.log');

date_default_timezone_set('Europe/Paris');

/*
 * Process stuff
 */

$params      = explode( '/', trim( key($_GET), '/' ) );
$paramsCount = count($params);

if ($paramsCount >= 2) {
  try {
    $controllerMethod = new ReflectionMethod( '\app\controller\\' .
                                                  $params[1], $params[2]
                                                );

    if ($paramsCount - 2 >= $controllerMethod->getNumberOfRequiredParameters()) {

      \ens\controller\Request::$lang =& $params[0];
      \ens\controller\Request::$data =& $_POST;

      $response = $controllerMethod->invokeArgs( null, array_splice($params, 3) );

    } else {
      $response = array('status' => 400);
    }

  } catch (ReflectionException $e) {
    $response = array('status' => 300);
  }

} else {
  $response = array('status' => 301);
}

if (!headers_sent()) {
  header('Content-Type: application/json');
}

echo json_encode($response);
share|improve this question
    
Have you benchmarked it? –  Jordan Jan 24 '12 at 23:09
    
You mean the class loader? –  Aalex Gabi Jan 24 '12 at 23:12
1  
Yeah, you ask if it's efficient enough. Have you run into performance problems that would suggest that it isn't? –  Jordan Jan 25 '12 at 1:15
    
The problem with benchmarking a class loader is that the class is loaded only once. So I don't know in what kind of loop to put it! I am thinking about a shell benchmark or generating some thousands random classes and loading them all? Can one of these work better than another? I saw that the default spl_autoload() is making stat-ing files before loading. –  Aalex Gabi Jan 25 '12 at 9:29

2 Answers 2

IMHO, all the ini_set doesn't belong here, but in the php.ini, unless you can't edit it for some reasons…

If so, it belong in a bootstrap file, as for the constant definitions and the include

A bootstrap file usually starts by including those libraries that are necessary for the file itself to function correctly. This involves setting general directory and file paths, loading configuration files, etc.

This bootstrap file can also be very useful for many reasons described on the link i provided like:

[…] set up different application statuses such as Development, Production or Testing […]

An other thing:

IMHO, the test if (!headers_sent()) is pretty useless as is.

As you are returning JSON, you need to send this header('Content-Type: application/json'), so, if you can't for any reason, you should raise an error or an exception, not just doing nothing and echoing your JSON as if nothing happened…

share|improve this answer

I don't think there are any useless parts. It's quite easy to read actually!

  1. No validation at all of $_GET and $_POST? It doesn't look like namespaces support relative navigations, but that seems a bit dangerous, except if you can Always Trust User Input.
  2. The "init stuff" could be put in another file and required.
  3. ÉNS is a bit weird for an application name (private joke, sorry).
share|improve this answer
    
First, thank you for review. 1. Q: What is relative navigation[namespaces]? 2. Good for styling. 3. ENS is a acronym. –  Aalex Gabi Mar 4 '12 at 17:15
    
1. \ens\controller\..\super_secret::DestroyEverything 3. I was thinking about the École Normale Supérieure, that's all. -- By the way, I don't have much to say, because I think it's quite good as it is. –  Quentin Pradet Mar 5 '12 at 7:23

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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