This repository has been archived by the owner. It is now read-only.

Allowing JDatabaseDriver::quote to take an array of strings. #1718

Merged
merged 2 commits into from Dec 4, 2012

Conversation

Projects
None yet
4 participants
@eddieajau
Contributor

eddieajau commented Nov 26, 2012

If supplied with an array of strings, the quote method will return an array of quoted strings. If neither a string nor array is passed to the quote method, an exception is thrown.
Added documentation for the escape and quote methods.
Updated unit test.

This avoids having to do something like the following every time you need to deal with an array:

$db = JFactory::getDbo();
$q = $db->getQuery(true);
$closure = function (&$v, $k) use ($q)
{
    $v = $q->q($v);
};
array_walk($strings, $closure);
Allowing JDatabaseDriver::quote to take an array of strings.
If supplied with an array of strings, the quote method will return an
array of quoted strings.
Documentation for the escape and quote methods added.
Unit test updated.
@dongilbert

This comment has been minimized.

Show comment
Hide comment
@dongilbert

dongilbert Nov 26, 2012

Contributor

+1 Very Useful

Contributor

dongilbert commented Nov 26, 2012

+1 Very Useful

@elinw

This comment has been minimized.

Show comment
Hide comment
@elinw

elinw Nov 26, 2012

Contributor

So a null is going to throw an exception?

Actually following up on that, this changes the handling of integers and booleans as well, which would have just been quoted as if they were strings before as far as I can tell (and I've made that mistake plenty of times). So I'm both tempted to say you should maintain the old api and quote them and even more tempted to be in favor of stricter typing and say why not also test is_bool, is_int, and is_float and not quote in those cases? Maybe only throw an exception if is_object.

Contributor

elinw commented Nov 26, 2012

So a null is going to throw an exception?

Actually following up on that, this changes the handling of integers and booleans as well, which would have just been quoted as if they were strings before as far as I can tell (and I've made that mistake plenty of times). So I'm both tempted to say you should maintain the old api and quote them and even more tempted to be in favor of stricter typing and say why not also test is_bool, is_int, and is_float and not quote in those cases? Maybe only throw an exception if is_object.

@eddieajau

This comment has been minimized.

Show comment
Hide comment
@eddieajau

eddieajau Nov 26, 2012

Contributor

Fair point. Fixed.

Contributor

eddieajau commented Nov 26, 2012

Fair point. Fixed.

@elinw

This comment has been minimized.

Show comment
Hide comment
@elinw

elinw Nov 27, 2012

Contributor

+1 Thanks

Contributor

elinw commented Nov 27, 2012

+1 Thanks

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Dec 2, 2012

I'd rather an option to disable type checking. PHP is loose about type casting for the convenience. For example, given the following contrived example:

class MyObject {

static $numObjects = 0

function __construct() {
self::$numObjects++;
}

function toString() {
return __CLASS
.'s';
}
}
$object = new MyObject;
$objectStats = array();
$objectStats['seperator'] = ' ';
$objectStats['number']=MyObject::$numObjects;
$objectStats['object'] = $object;
$objectStats['msg'] = 'have been created. -- Percent declared classes:';
$objectStats['percentage'] = (1/count(get_declared_classes()))*100;
$objectStats['msg2'] = '-- Objects Exist flag';
$objectStats['exists'] = ($numObjects) ? true: false;

// store the stats into the database
$checkType = false;
$sqlUpdate = ' set objectStats = CONCAT_WS( implode(',', JDatabaseDriver::quote($updateStrings, $checkType));
$sqlUpdate .= ', objectNumber='.$objectStats['number'];
$sqlUpdate .= ', objectMetrics='.$objectStats['percentage];
$sqlUpdate .= ', objectName='.JDatabaseDriver::quote($objectStats['object]);

So with one array of data it will assign an integer to objectNumber, a float to objectMetrics, a string to objectName[via the magic __toString method and the quote method], and an informational string to objectStats

The first line becomes:
CONCAT_WS(' ', '1', 'MyObjects', 'have been created. -- Percent declared classes:', 'nn.nnn', '-- Objects Exist flag', '1');

Which MySQL then converts to:
'1 MyObjects have been created. -- Percent declared classes: nn.nnn -- Objects Exist flag 1'

Granted, I'm not really thrilled with the readability of the above but I certainly have seen code which uses lots of magic methods and such to create things programatically. Make the checkType flag default to true and then programmers always have the option of overriding it without writing their own subclass.

ghost commented Dec 2, 2012

I'd rather an option to disable type checking. PHP is loose about type casting for the convenience. For example, given the following contrived example:

class MyObject {

static $numObjects = 0

function __construct() {
self::$numObjects++;
}

function toString() {
return __CLASS
.'s';
}
}
$object = new MyObject;
$objectStats = array();
$objectStats['seperator'] = ' ';
$objectStats['number']=MyObject::$numObjects;
$objectStats['object'] = $object;
$objectStats['msg'] = 'have been created. -- Percent declared classes:';
$objectStats['percentage'] = (1/count(get_declared_classes()))*100;
$objectStats['msg2'] = '-- Objects Exist flag';
$objectStats['exists'] = ($numObjects) ? true: false;

// store the stats into the database
$checkType = false;
$sqlUpdate = ' set objectStats = CONCAT_WS( implode(',', JDatabaseDriver::quote($updateStrings, $checkType));
$sqlUpdate .= ', objectNumber='.$objectStats['number'];
$sqlUpdate .= ', objectMetrics='.$objectStats['percentage];
$sqlUpdate .= ', objectName='.JDatabaseDriver::quote($objectStats['object]);

So with one array of data it will assign an integer to objectNumber, a float to objectMetrics, a string to objectName[via the magic __toString method and the quote method], and an informational string to objectStats

The first line becomes:
CONCAT_WS(' ', '1', 'MyObjects', 'have been created. -- Percent declared classes:', 'nn.nnn', '-- Objects Exist flag', '1');

Which MySQL then converts to:
'1 MyObjects have been created. -- Percent declared classes: nn.nnn -- Objects Exist flag 1'

Granted, I'm not really thrilled with the readability of the above but I certainly have seen code which uses lots of magic methods and such to create things programatically. Make the checkType flag default to true and then programmers always have the option of overriding it without writing their own subclass.

@eddieajau

This comment has been minimized.

Show comment
Hide comment
@eddieajau

eddieajau Dec 3, 2012

Contributor

@garyamort, I don't understand what $updateStrings is in the example.

$sqlUpdate = ' set objectStats = CONCAT_WS( implode(',', JDatabaseDriver::quote($updateStrings, $checkType));

Contributor

eddieajau commented Dec 3, 2012

@garyamort, I don't understand what $updateStrings is in the example.

$sqlUpdate = ' set objectStats = CONCAT_WS( implode(',', JDatabaseDriver::quote($updateStrings, $checkType));

@elinw

This comment has been minimized.

Show comment
Hide comment
@elinw

elinw Dec 3, 2012

Contributor

See #1733 for some new tests.

Contributor

elinw commented Dec 3, 2012

See #1733 for some new tests.

LouisLandry added a commit that referenced this pull request Dec 4, 2012

Merge pull request #1718 from eddieajau/database
Allowing JDatabaseDriver::quote to take an array of strings.

@LouisLandry LouisLandry merged commit b584e04 into joomla:staging Dec 4, 2012

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.