saguaroib / saguaro

saguaro imgboard software
14 stars 5 forks source link

Refactor - Error handling, performance profiling #198

Closed Apogate closed 6 years ago

Apogate commented 8 years ago

After the completion of Regist's conversion, I think it's time for some proper error handling and some basic performance profiling. Basically, making debug mode more potent than just allowing PHP error reporting.


Error To do:

Profiling To Do:

Apogate commented 8 years ago

It would actually make sense to convert the language/string files to variable-array. Cuts down on "costly" defines and has the added benefit of being overwriteable.

RePod commented 8 years ago

The problem with switching off constants (which itself is just a proxy) is that they'd be inaccessible without using global to include them in the local scope every time we need them.

Also due to your new E_ strings, they're currently not defined by the proxy. However, I do not recommend using E_ as PHP uses that prefix for its own errors.

Possible solutions:


My previous class-based solution for a situation like this was for the config:

<?php
$config = array();
$config['aye'] = [ 'lmao' => 'xd' ];
$config['webm'] = [
    'allow' => false,
    'maxduration' => 30,
    'audio' => true
];

var_dump($config); //Dumps current config.

class SaguaroConfig {
    private $config = array();
    function init($config) {
        if ($config && empty($this->config)) {
            $this->config = $config; //Store once.
        } else {
            return "Some problem with config init.";
        }
    }
    function get($key = null) {
        return ($key) ? $this->config[$key] : $this->config;
    }
}

$backup = new SaguaroConfig; $backup->init($config);
unset($config); var_dump($config); //null
var_dump($backup->get()); //Dumps copied config contents.
?>

The problem though is unlike constants it's really difficult to group them and automate them like the current config file does so well (aside from the fact they cannot be changed later in execution).

Also it would effectively require two classes: Languages and Error. Languages to contain the strings and Error which fetches them from the Languages class. The benefit though is we can modify both freely to produce the results we want across the software (like having Language replace all "aye"s with "lmao").

A disadvantage to this is, given my example class, it's read-only. However this shouldn't be an issue because if we really wanted to change it we could (hopefully securely) read it out and use a modified version of what we want.

Another disadvantage to this, unlike classes, is it's not true read-only. At least in its current form. There are two ways to combat it being reinitialized (by literally creating a new instance):

RePod commented 8 years ago

Also I think you're abusing milestones.

I updated my previous comment with my last proposed solution (albeit for config, it still works).