I have a function in a module I'm porting that's basically like this:

function module_some_form_submit($form_id, $form_values) {
  if ($form_values['blah']) {
    print ($form_values['other_blah']);
  }
  else {
    print ($form_values['still_other_blah']);
  }
}

Coder correctly dings me on the fact that the submit function arguments have changed. Great!

It also dings me 3 times that $form_values has changed to $form_state['values']. Great! ...at first.

The least invasive way to fix this problem is to put this single line at the top of the function:

$form_values = $form_state['values'];

and then leave the rest of the code alone.

However, Coder doesn't understand that /my/ $form_values has now accounted for $form_state['values'], and continues to flag this as an error. The only way to get it to stop is to rename all of the $form_values arrays to like $input or $values or something else. This is prone to errors during manual replacement. Especially considering I have about 10 of such functions. ;)

Any ideas on how best to handle this? I'm torn, because the warning is great and successfully catches a real problem, but the way to fix it is a bit of a sledgehammer.

Comments

douggreen’s picture

douggreen commented

I don't know a solution.

You're one line fix is simple from a porting perspective, but I'm not sure what php does with the statement. If it's copying the entire array, it is a little wasteful from a cpu cycle perspective. Also, you're one line fix might be a little error prone, since $form_values was pass by reference, you're new $form_values won't be pass-by-reference, so you should really check each usage to make sure you're not setting the value.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

Ah, perhaps. I assumed it was faster to go get $form_values['foo'] rather than $form_state['form_values']['foo'] all the time. Only one index to retrieve. If that's not the case, then feel free to won't fix.

douggreen’s picture

douggreen commented
Status: Active » Closed (won't fix)

It probably is faster to do a single array index lookup, but I'm thinking that it might be slower to copy the entire $form_state['values'] array to $form_values, especially if you have lots of form elements. php might handle this gracefully, but I don't know. Either way, I think that this is going to have to be a "won't fix", because it's kinda hard.