Clearing up global constants
Issue #210
resolved
Originally reported on Google Code with ID 210
Do we need DATATYPE, FUCTIONS and CUSTOM_FUCTIONS?
Datatypes simply stores an array, used a few times later in the code:
index.php:96: define("DATATYPES", serialize($types));
index.php:1326: $types = unserialize(DATATYPES);
index.php:2644: $types = unserialize(DATATYPES);
index.php:2756: $types = unserialize(DATATYPES);
Functions is similar, and is only used together with CUSTOM_FUNCTION, through getUserFunctions:
index.php:100: define("FUNCTIONS", serialize($functions));
index.php:2240: $functions = array_merge(unserialize(FUNCTIONS), $db->getUserFunctions());
index.php:2345: $functions = array_merge(unserialize(FUNCTIONS), $db->getUserFunctions());
This last one confuses me a bit...
index.php:101: define("CUSTOM_FUNCTIONS", serialize($custom_functions));
classes/Database.php:41: $cfns = unserialize(CUSTOM_FUNCTIONS);
classes/Database.php:53: $cfns = unserialize(CUSTOM_FUNCTIONS);
classes/Database.php:66: $cfns = unserialize(CUSTOM_FUNCTIONS);
Custom/user functions are added to the database object in Database::__construct, but
then a copy is store also in Database. No other use is made in the Database class,
but they are requested from the main flow through getUserFunctions.
A patch is attached that would:
- dispose of DATATYPE and FUNCTIONS, renaming the corresponding arrays as $sqlite_datatypes
and $sqlite_functions for clarity
- remove the code relative to CUSTOM_FUNCTIONS from Database::__construct
- remove addUserFunction and getUserFunctions from Database
- add an equivalent Database::registerUserFunction
- call registerUserFunction on database creation
Result: fewer constants, less data stored in Database, fewer function calls to move
this data around.
Please double check this, since I might have missed something important :)
Reported by dreadnaut
on 2013-04-06 19:57:22
<hr> * Attachment: constants.diff
Comments (7)
-
-
reporter Right! We should register custom functions also when importing data, at least. Attached is a new patch to fix that, and the two rogue spaces. We still have 'FORCETYPE' and 'PAGE' around, but I'm leaving them for later. I don't feel confortable hacking at the Database class yet, but I'd like to clean up the constructor and remove all output from the class. Also, there are a lot of 'if($this->type==' around...
Reported by
dreadnaut
on 2013-04-24 10:32:49 - Status changed:Started
<hr> * Attachment: constants.diff
-
Looks good. Feel free to commit it.
Reported by
crazy4chrissi
on 2013-04-26 19:16:33 -
reporter This issue was closed by revision r412.
Reported by
dreadnaut
on 2013-04-30 11:40:35 - Status changed:Fixed
-
Thanks.
Reported by
crazy4chrissi
on 2013-04-30 11:45:42 -
reporter Thank you for checking my code!
Reported by
dreadnaut
on 2013-04-30 11:46:54 -
Reported by
crazy4chrissi
on 2014-01-02 16:26:41 - Labels added: Target-1.9.5 - Log in to comment
Reported by
crazy4chrissi
on 2013-04-22 22:15:58