0
\$\begingroup\$

I wanted to display 1 outcome of 1 single custom field or attribute in the category overview, using a third party customfield extension in my J! and VM website. I got it working using the code below. As this is the first time I actually write PHP and MySQL stuff myself, I was pretty proud, but also thinking 'this can probably be done better..'

<?php  
//first get the customfieldid for the product where the custom id is 10
$db2 = JFactory::getDBO();
$db2->setQuery("SELECT virtuemart_customfield_id FROM #__virtuemart_product_customfields WHERE virtuemart_product_id=".$product->virtuemart_product_id." and virtuemart_custom_id=10 ;");
$db2->query();
$customFieldId = $db2->loadResult();

//make a database connection and find the field record that contains the customfield string
$db = JFactory::getDBO();
$db->setQuery("SELECT customsforall_value_id FROM #__virtuemart_product_custom_plg_customsforall WHERE virtuemart_product_id=".$product->virtuemart_product_id." and customfield_id=".$customFieldId." ;");  
$db->query();
$staatValueId = $db->loadResult();

//knowing the customsforall id with the product, saved in a $ we get the attached string
$customStaat = JFactory::getDBO();
$customStaat->setQuery("SELECT customsforall_value_name FROM #__virtuemart_custom_plg_customsforall_values WHERE virtuemart_custom_id=10 and customsforall_value_id=".$staatValueId." ;");
$customStaat->query();
$resultStaat = $customStaat->loadResult();

//display the state
echo "<span class=\"cat-staat-product\">".$resultStaat."</span>";
?>

Can I do this better, or clean things up?

\$\endgroup\$
  • 1
    \$\begingroup\$ Please state only the purpose of the code in the title. \$\endgroup\$ – Caridorc Apr 7 '15 at 12:22
1
\$\begingroup\$

Security

You should use prepared statements for all variable data to protect against SQL injection.

The thing is, Joomla actually doesn't seem to support prepared statements. For me, that would be a reason to think about using a different CMS, but you should at the very least follow their guidelines to prevent SQL injection (checking data type and escaping. which - when done correctly - does offer complete protection against injections).

When echoing data, you should also protect against XSS.

Misc

  • all your SQL keywords should be uppercase (eg and -> AND) to increase readability.
  • your table and column names are too long, which makes the queries hard to read. For example, to you really need customsforall or virtuemart in them?
  • you get a new DBO for each query, this doesn't seem necessary.
  • you should be able to use JOIN to reduce the amount of queries you have.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.