[drupal-devel] Checking coding style of contributed modules

Gerhard Killesreiter killesreiter at physik.uni-freiburg.de
Sun Feb 20 14:55:43 UTC 2005



On Sun, 20 Feb 2005, Dries Buytaert wrote:

> Gerhard Killesreiter wrote:
> >
> > On Sat, 19 Feb 2005, Dries Buytaert wrote:
> >
> >
> >>I slapped together the foundations of a code checker scripts and
> >>installed it on drupal.org.  It took me 50 minutes to write and install.
> >>  It catched hundreds of (small) issues already, and will continue to
> >>catch many more in future to it most certainly pays off.
> >
> > talk == silver
> > code == gold. :)
>
> I installed the updated version on drupal.org!  Thanks.

Thanks. :)

Now we will get complaints about db queries that look like these:

db_query("INSERT INTO {node} (". implode(", ", $k) .") VALUES(". implode(", ", $s) .")", $v);

db_query("UPDATE {node} SET ". implode(', ', $q) ." WHERE nid = '$node->nid'", $v);

The $node->nid in the last one is easy to fix, but the implodes for SET
or VALUES aren't unless we simply assume that all fields need to be
updated/inserted. This would be possible in this case (and require some
additional changes for the update case) but not in other cases where
similar constructs are used.

The question is: can we extend our db wrapper to support a syntax like

db_query("INSERT INTO {node} %a", $kv);

Where $kv would contain $k as keys and $v as values.

db_query("UPDATE {node} SET %a WHERE nid = %d", $kv, $node->nid);

Note that %a is not supported in sprintf and we'd need to roll our own.

I recall that Steven had some idea into this direction. I also recall
that I wasn't too thrilled at the time he proposed it. ;-)
But I realize know that it is just carrying our db abstraction layer a
step further.

Cheers,
	Gerhard




More information about the drupal-devel mailing list