Before:
Code: Select all
static function extract_current_page($root_path)
{
global $request;
$page_array = array();
// First of all, get the request uri...
$script_name = htmlspecialchars_decode($request->server('PHP_SELF'));
$args = explode('&', htmlspecialchars_decode($request->server('QUERY_STRING')));
if (!$script_name)
{
$script_name = htmlspecialchars_decode($request->server('REQUEST_URI'));
$script_name = (($pos = strpos($script_name, '?')) !== false) ? substr($script_name, 0, $pos) : $script_name;
$page_array['failover'] = 1;
}
...
Code: Select all
static function extract_current_page($root_path)
{
global $request;
$server = $request->server;
return self::extract_current_page_($root_path, $server('PHP_SELF'), $server('QUERY_STRING'), $server('REQUEST_URI'));
}
static function extract_current_page_with_variables($root_path, $script_name, $query_string, $request_uri)
{
$page_array = array();
// First of all, get the request uri...
$script_name = htmlspecialchars_decode($script_name);
$args = explode('&', htmlspecialchars_decode($query_string));
if (!$script_name)
{
$script_name = htmlspecialchars_decode($request_uri);
$script_name = (($pos = strpos($script_name, '?')) !== false) ? substr($script_name, 0, $pos) : $script_name;
$page_array['failover'] = 1;
}
...
- Decoupled means $request->server is easier to change in the future
- Less global state in the function means input/output is more predictable
- By simply splitting out the state but keeping the same interface ensures compatibility
- Easier to write tests, function inputs map to outputs (with no side-effects)
- More comprehensive tests, since some global objects are read-only
Cons:
- More lines of code (bloats class)
- Pollutes name space with functions that end with _with_variables (or _underlying or __)
- Tests that I write will require this these changes to work
Alternatively I could just try to write tests with the existing functions, but they will not be as comprehensive.