While updating database queries from D6 to D7 in a not fully properly ported and tested Drupal 7 module - this below is an example of a query that leaves an undefined variable $nid in this case on line 5 - I have a lot of these kinds of problems with the queries leaving undefined variable errors now because the D7 query does not return an associative array any longer - only objects. Is there a proper standard coding example on how to fix ???

I have fifteen "critical" modules I will have to convert myself because the original developer / maintainer is either unresponsive to support, abandoned Drupal and the module, or produced buggy code that has these kinds of db_result and db_fetch errors and calls STILL in the module.

Because this is an "if" query in a group of nested "if' statements - can someone tell me why a foreach loop would even be needed ??

The original D6 query was:

 if (db_result(db_query("SELECT * FROM {uc_recurring_stripe} WHERE nid = %d", $item['nid']['#value']))) { 

I tried to change it to - as line 2 below - but that generates a parse error of unexpected T operator at the fetchAssoc

 if (isset($item['nid']['#value'])) {
 if (db_query('SELECT * FROM {the_table} WHERE nid = :nid', array(':nid' => $item['nid']['#value'])))->fetchAssoc();               
 $title = function_name($item['nid']['#value']);
 if (isset($title)) {
 $form['items'][$key]['desc']['#value'] = l($title, 'node/' . $nid);

I also tried

 if (db_query('SELECT * FROM {the_table} WHERE nid = :nid', array(':nid' => $item['nid']['#value']), array(':nid' => $nid))) {

That threw a general error into the browser of "the website has experienced an error try again later", which was actually in the log a,

Recoverable fatal error: Object of class DatabaseStatementBase could not be converted to string in function_name() (line 131 of /name.module).

Any iteration of this query creates an undefined variable of $nid at the $form line (5) above

I silenced the error by pre-defining the variable ABOVE the query as

          if (isset($item['nid']['#value'])) {
            $nid = [ ];
	    if (db_query('SELECT * FROM {the_table} WHERE nid = :nid', array(':nid' => $item['nid']['#value']))) {
              if ($node = menu_get_object()) {
              // Get the nid
              $nid = $node->nid;
              $title = function_name($item['nid']['#value']);

And calling menu_get_object() BELOW the query as shown above

I am not sure if that is the best way to do this and if the code will even work properly

I have a few other undefined variable issues to kill below other converted queries before I can see if I just screwed the module all up instead OR actually fixed the issues - I am killing errors that show in the dblog not single stepping with an IDE like Netbeans etc.

Is the "if"statement as to simple static queries GONE from Drupal 7 as well and ALL queries must be re-written to foreach loops or pre-defining a variable and then calling it later as i have done above??

I realize something like this should also work

 $nid = [ ];
 $result = db_query('SELECT n.nid, n.title, n.created FROM {the_table} n  WHERE n.nid = :nid', , array(':nid' => $item['nid']   ['#value']));
 // Result is returned as a iterable object that returns a stdClass object   on each iteration
 foreach ($result as $record) {
   // Perform operations on $record->title, etc. here.
   // in this example the available data would be mapped to object properties:
   // $record->nid, $record->title, $record->created
 }

BUT . . . is this complication now the ONLY way to do this to define the variable for line 5 from the above example query ??

Can someone provide what might be the best accepted coding standard in Drupal 7 to do this kind of code re-adjustment to fix an undefined variable

Comments

Jaypan’s picture

There is a problem with the original code:

 if (db_result(db_query("SELECT * FROM {uc_recurring_stripe} WHERE nid = %d", $item['nid']['#value']))) { 

The result will be unpredictable. In D6, db_result() returns the first column of the first row found. However, the column is unknown, because the query is selecting *, rather than a column name. The fact that this actually worked at some time is luck more than anything, as it's not a stable query whatsoever.

I'm guessing that the author was trying to determine whether or not a row exists in the table for the given NID. Under that assumption, I'd convert the above query to the following:

$exists = db_query('SELECT 1 FROM {uc_recurring_stripe} WHERE nid = :nid', array(':nid' => $item['nid']['#value']))->fetchField();
if($exists)
{
  // do something.
}

Note that as non-zero numbers evaluate to TRUE, and this query is selecting 1 if a row exists for the given nid, when a row exists fetchField() will return 1 (which evaluates to TRUE), and when a row doesn't exist, fetchField() will return FALSE (default behavior for empty results). Therefore $exists will be equal to either 1 (TRUE) or FALSE.

-------------------------
Looking at your other code snippets:

 if (db_query('SELECT * FROM {the_table} WHERE nid = :nid', array(':nid' => $item['nid']['#value'])))->fetchAssoc();               

You are calling fetchAssoc() on your if statement, not the db_query. Check/count your brackets.

 if (db_query('SELECT * FROM {the_table} WHERE nid = :nid', array(':nid' => $item['nid']['#value']), array(':nid' => $nid))) {

Why are you passing the third argument (array(':nid' => $nid))? The third argument to db_query() is supposed to be an array of options. You have passed it an array of replacements for the placeholder.

$result = db_query('SELECT n.nid, n.title, n.created FROM {the_table} n  WHERE n.nid = :nid', , array(':nid' => $item['nid']   ['#value']));

This is incorrect, and should have thrown an error, since you have not passed anything for the second argument (see your double commas).

To answer your questions:

Is the "if"statement as to simple static queries GONE from Drupal 7 as well and ALL queries must be re-written to foreach loops or pre-defining a variable and then calling it later as i have done above??

No. You could do this, and it would work fine:

if(db_query('SELECT 1 FROM {the_table} WHERE nid = :nid', array(':nid' => $item['nid']['#value']))->fetchField())
{
  // do something
}

Though I prefer to do it the way I showed earlier in this post, as it makes your code easier to read, and clearer to understand what is happening.

bobburns’s picture

I looked at that query and flow and it did not look right . . . and while I did not know exactly how to fix it - the question was based exactly on my suspicions. I indeed it appeared that query did nothing but return for the if statement

The two commas was only one when I tried it - I must have pasted the extra one in by error when I built the question

The variable as I declared it seems to be working - and now I run into bad and missing code I can tell has to be re-written because a JSON response is missing from the returned $data in the array as set up

Trying to figure out another's mindset in this code is not something I expected I would have to do, but so much of it is non-functional I am going to have to break certain parts of it to force execution of others to see what the result and flow is then

At least i am farther along and it is working on the side that matters

Jaypan’s picture

No, problem, you're welcome.

...oh, wait.

bobburns’s picture

Jaypan:

Drupal 7 pulled off a very big band aid and the bleeding is ugly. I get the reasons for the changes - as i see the improvements and sadly am running into the sloppy coding that was going on in Drupal 6. Drupal 7 is faster - the new developments when used are great - but many developers are throwing in the towel - or will not respond to support their projects anymore.

You have made me see now why I am looking at a substantial re-write of this mess I have in this as a module from another mother. Now that i actually sat down and read the code - I say OMG. Plenty of sub-routines and blocks that do not run as they are never called, calls that are not needed as routines, calls improper for the new hook schema and general logic that is truly a hack.

Now that you have introduced me to how clean queries should go - and play nicer with the new Drupal 7 hook schema; sadly many like me do not share the same enthusiasm - and only see broken code that once worked fine because it is Drupal 7 to them that broke it.

Drupal 6 or 7 is the KING of the CMS's but it is killing its own soldiers and software extensions in the modules and the army may end up dwindling and that is not good. Drupal 8 will be esoteric and may have no army left. I believe i saw somewhere that Christefano said and compared that Drupal 6 is comparable to something like Windows XP and implying Drupal 7 is limping towards being a Vista that may convert itself into Windows 7 slowly. See https://groups.drupal.org/node/291243 So . . . . he implies . . . where does that leave Drupal 8 ??

Even worse there are STILL those who know Drupal 6 coding ways - who want to chime in on various support sites and they are spouting Drupal 6 coding that does not work on Drupal 7 and do not even know it, yet it gets indexed on the web search engines just as equally - making for "spaghetti support" - or worse snippy, snarky replies to questions perplexing so many trying to use Drupal 7 like "look stupid you should know that" and a point to a link - sometimes irrelevant (NOT YOU OF COURSE !!!)

But the real battle is at the ISP and Hosting company level - who just will not - or cannot hire Drupal 7 people - or cannot afford to support Drupal 7 - because it just generates TOO MANY questions and looks buggy and has silliness issues their customers will leave them over - just like the ones you answered many times the COMMON guy will ONLY call a "BUG" - because that is ALL they know and FEEL they should call it.

I have a small CPanel/WHM managed VPS system with SSD architecture running the 64 bit CentOS and it is just what Drupal 7 needs - but many people will want to try cheap Volkswagen hosting accounts that will just choke on issue after issue of PHP version and memory issues and database issues - when putting Drupal 7 under even a moderate load because the ISP or Hosting company has restricted the server AND no one there knows Drupal 7 issues and needs. Those SSD's make a huge difference on this query heavy Drupal 7 platform - especially if dynamic queries are more heavily used in the future and for instance I needed to be able to up my memory myself to get the Coder module(s) to run - not to mention installing Drush, Codesniffer, Composer and Xdebug - that many cheap hosting companies will not do as in an install for you.

Then just ONE critical module like this one I have been working on for over a month - is enough. It is running - but limping. I now see the ugliness in this code and I know I should not have had so much faith in ANY module just being coded well and working right.

There are a handful of guys like you Jaypan who are worth their weight in gold because they really know what they are talking about - and they - and you - must be over loaded with kindergarten questions because it is Drupal 7 breaking working Drupal 6 code.

So . . . i do sincerely appreciate the time you have taken to support Drupal 7 and its issues after the powers that be made the decision of ripping off that very big band-aid on Drupal 6.

So . . . Oh . . . wait . . . you have been a lifesaver !!!

Jaypan’s picture

Glad to have helped.

WorldFallz’s picture

don't confuse the baby with the bathwater.

There are tons of shoddy, shady, lazy programmers (I won't call them developers) who are nothing more than charlatans that know how to code. You'll encounter them everywhere, with every language. Rather than taking pride in their work, they're only interested in writing just enough code to get their paycheck then go off to the next poor snook. They're not in interested in learning something new or doing it the right way. They only want to get it done as fast as possible to get on to the next job-- they know no one is likely to call them back to fix their own mess. They count on it actually. And usually make themselves difficult to contact once the bulk of the payment has changed hands.

In the php world it's usually php jockies who think that because they know php they are also now expert codeignitor, wordpress, joomla, symfony, laravel, drupal, <insert new hot php framework here>, developers. They are not.

I think it may seem to happen a lot in drupal because it's harder to fake your way through drupal than it is in some of those other frameworks. Each major version of drupal will rip the bandage off anew. It's not that other frameworks/apps don't have them, but they may just stay hidden longer because you don't have revisit custom code every time you do a major version upgrade.