DB_escape_string

[serakfalcon]

The code I posted before requires the DB change below, as it no longer passes a time value but lets the DB set the timestamp on insert.

the parameters:

Code:

($query,
                    $bindvars = null,
                    $ErrorMessage='',
                    $DebugMessage= null,
                    $Transaction=false,
                    $TrapErrors=true)

Code changes: &$Conn is no longer passed instead the global $db is used inside the function.
Every value to be added to the DB should be replaced by a ?, and instead added to the array $bindvars in order of appearance in the code.
so $bindvars[0] represents the first question mark, $bindvars[1] the second and so on. I recommend using an associative array of array('columnname' => 'value') which will help any future implementation of PDO but it's not required.
All the other parameters work the same as before, except there are new errors if the statement could not be prepared and variables could not be bound.

The return values will be an associative array of all values returned by the query, the first key being row and the second key being by column name (in order as per the statement)
so for example in

Code:

SELECT transactiondate, userid, querystring
FROM audittrail LIMIT 5

$returnvalue[0]['transactiondate'] will be the transactiondate of the first column, but it could also be accessed as $returnvalue[0][0].
So essentially we replace the while loops with a foreach loop.

Instead of pre-fetching all elements into an array, we could pass the statement as the result of the function, however if we do that then we need to remember to close the statement after we're done with it. in this case it would be

Code:

while($myrow = $returnvalue->fetch()) {
//do stuff
} //end while
$returnvalue->close();

I don't like the idea of relying on people to close things so that's why an array, but it is a matter of preference and is simple to alter.

Other notes:
The audit trail stores the params as JSON which strikes a balance between being able to be recalled by the code, and being human readable (the alternative would be to iterate through the array and replace '?' with each variable, but it won't work properly if question marks are included in any fields (for example if we had field notes and inserting the comment 'Follow up?', the next string replace call will replace that question mark, so either we write some sort of parser or come up with a regexp that we are certain will not come up as input, or just keep them separate.)
So that's why JSON. Naturally, any code that displays the audit trail will need to show the new params column, so that would be another change.

Code:

ALTER TABLE `audittrail`
CHANGE COLUMN `transactiondate` `transactiondate` TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP ,
ADD COLUMN `params` TEXT NULL AFTER `querystring`;

As a side note on Doctrine DBAL, you can write it almost exactly like a PDO statement, except that it supports many different DB types out of the box, so while I do prefer abstraction to make things easier in general, in this case Doctrine DBAL doesn't abstract much as far as the queries are concerned, but automatically converts the query to fit the particular database you want, which would expand the amount of DB's supported by webERP significantly at negligible cost. With the classloader, it only loads classes you use so it is fairly lightweight. In this case we could even use named parameters with the mysqli driver!

---

As previously noted and I remain unconvinced, I am not keen on adding frameworks or abstracting tables to a class - which to my mind provide only limited advantages at the cost of readability.

You have very strange opinions but I will respect them in this case regardless.