[Vtigercrm-developers] sql programming error / coding standards

[Adam Heinz]

Excerpt from include/Webservices/VtigerCRMObjectMeta.php:111-120

$sql = 'select * from vtiger_profile2tab where profileid in
('.generateQuestionMarks($profileList).') and tabid = ?;';
$result = $adb->pquery($sql,array($profileList,$this->getTabId()));
$standardDefined = false;
$permission = $adb->query_result($result,1,"permissions");
if($permission == 1 || $permission == "1"){
$this->hasAccess = false;
return;
}else{
$this->hasAccess = true;
}

The problem that I'm having here is that one of my roles has three
associated profiles, but this logic looks at only the first and throws the
rest away.  I haven't found this bug in Trac yet.

This sort of programming error is potentially systemic in the vtiger
codebase.  It is very common to see $adb->pquery used to query the
database, but then only one row is ever used without looking at the row
count of the response.  I am a big fan of ADOdb (which PearDatabase wraps),
so rather than ask for all rows and post-process with PHP, I ask the entire
question in SQL.  I also pierce the abstraction layer to get at the exact
ADOdb method that fits this situation.

$sql = 'select count(1) from vtiger_profile2tab where permissions = 0 and
profileid in ('.generateQuestionMarks($profileList).') and tabid = ?';
$standardDefined = false;
$permission = $adb->database->GetOne($sql,
$adb->flatten_array(array($profileList,$this->getTabId())));
if (!$permission) {
$this->hasAccess = false;
return;
}else{
$this->hasAccess = true;
}

I greatly prefer self-describing code (no comments) and direct access to
the ADOdb object lets me clearly GetOne, GetRow, GetCol, GetAll, etcetera.
 General opinions on direct ADOdb access versus "PearDatabase"?  I don't
quite understand why we have a db abstraction layer wrapping a db
abstraction layer.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.vtigercrm.com/pipermail/vtigercrm-developers/attachments/20120116/a28834a2/attachment-0002.html