At a quick glance without any planning or knowing a whole lot about joomla, I'd
be inclined to make Classes for #__vm_product_selecon_product_codes and
#__vm_product set custom_attribute in this case.
That way, the code could look more like this when used:
// New usage code - The reusable part
$myCodes = new vm_product_selecon_product_codes();
$list = $myCodes->getAll();
$myProductAttributes = new vm_product set custom_attribute()
$myProductAttributes->updateAttributes($list);
// getAll() method code
$db =& JFactory::getDBO();
$sql = 'select product_id, product_sku from
#__vm_product_selecon_product_codes '.
'order by product_id, product_sku ';
$db->setQuery($sql);
$list = $db->loadAssocList();
return $list;
// updateAttributes() method code
$db =& JFactory::getDBO();
$last_pid = 0;
$first = 1;
$imp=array();
if ($list){
foreach ($list as $entry){
if ($entry['product_sku'] && !in_array($entry['product_sku'], $imp)){
$imp[] = $entry['product_sku'];
}
if (!$first){
if ($last_pid != $entry['product_id']){
$upsql = "update #__vm_product set
custom_attribute='".implode(';',$imp).
"' where product_id=".$last_pid;
$db->setQuery($upsql);
$db->query();
$imp=array();
$last_pid = $entry['product_id'];
}
}else{
$first = 0;
}
}
}
// Nothing needed to return by the looks. Maybe true/false?
----- Original Message -----
From: Jochen Daum
To: [email protected]
Sent: Thursday, February 19, 2009 11:08 AM
Subject: [phpug] Re: can this be done more elegantly
Hi Aaron,
On Thu, Feb 19, 2009 at 11:03 AM, Aaron Cooper <[email protected]> wrote:
Hi Jochen,
Just a performance thing rather than elegance:
Does that DB layer support prepared statements?
$upsql = "update #__vm_product set custom_attribute='".implode(';',$imp).
"' where product_id=".$last_pid;
Might see some benefit being a prepared statement if it's fired quite a few
times in that foreach loop.
yes, good idea, I'm not sure about prepared statements. In this case I would
also like it with less memeory impact, hence just reading row by row, but it
doesn't support that.
But not the elegance question either.
Jochen
Aaron
----- Original Message -----
From: Jochen Daum
To: PHPUG
Sent: Thursday, February 19, 2009 10:54 AM
Subject: [phpug] can this be done more elegantly
Hi,
I have a common piece of code below and I wonder if this can be done more
elegantly. The code below reads records from a database - in this example a
product_id and lots of skus that represent variations of this product. It then
implodes all skus of a product into a field in another table. This is done for
each product_id.
I know this can be done with less lines of code by using @, condensing if
statements and accepting that it throws E_NOTICE some times. But aside from
that, can it be done completely diferently and more elegantly.
I'm seeing myself writing this piece of code over and over, covering
database queries or converting one array structure into another.
BTW, database layer is from Joomla, but could be any database layer.
Cheers,
Jochen
$db =& JFactory::getDBO();
$sql = 'select product_id, product_sku from
#__vm_product_selecon_product_codes '.
'order by product_id, product_sku ';
$db->setQuery($sql);
$list = $db->loadAssocList();
$last_pid = 0;
$first = 1;
$imp=array();
if ($list){
foreach ($list as $entry){
if ($entry['product_sku'] && !in_array($entry['product_sku'],
$imp)){
$imp[] = $entry['product_sku'];
}
if (!$first){
if ($last_pid != $entry['product_id']){
$upsql = "update #__vm_product set
custom_attribute='".implode(';',$imp).
"' where product_id=".$last_pid;
$db->setQuery($upsql);
$db->query();
$imp=array();
$last_pid = $entry['product_id'];
}
}else{
$first = 0;
}
}
}
}
--~--~---------~--~----~------------~-------~--~----~
NZ PHP Users Group: http://groups.google.com/group/nzphpug
To post, send email to [email protected]
To unsubscribe, send email to
[email protected]
-~----------~----~----~----~------~----~------~--~---
