E_ALL Compliance: Fix PHP Notices in issue.inc
plumbley - December 17, 2006 - 13:51
| Project: | Project issue tracking |
| Version: | 5.x-2.x-dev |
| Component: | Issues |
| Category: | bug report |
| Priority: | minor |
| Assigned: | Unassigned |
| Status: | closed |
Description
issue.inc produces various PHP Notices with E_ALL on. These are all due to variables, array keys or object fields which may not exist when used.
This patch fixes those that I spotted, by changing to equivalent behaviour without Notices.
Examples:
1.
<?php
switch ($_POST['op'] ? $_POST['op'] : arg(2)) {
?>changed to
<?php
switch (!empty($_POST['op']) ? $_POST['op'] : arg(2)) {
?>2.
<?php
'#default_value' => $status[$nid],
?>changed to
<?php
'#default_value' => isset($status[$nid]) ? $status[$nid] : NULL,
?>3.
<?php
$pid = $node->pid ? $node->pid : $_POST['edit']['pid'];
?>changed to
<?php
$pid = !empty($node->pid) ? $node->pid : (isset($_POST['edit']['pid']) ? $_POST['edit']['pid'] : NULL);
?>4.
<?php
if (trim($project->help)) {
?>changed to
<?php
if (isset($project->help) && trim($project->help)) {
?>5.
<?php
'#default_value' => ($query->states) ? $query->states : array(1, 2, 8, 13, 14),
?>changed to
<?php
'#default_value' => !empty($query->states) ? $query->states : array(1, 2, 8, 13, 14),
?>Best wishes,
Mark.
| Attachment | Size |
|---|---|
| issue.inc_3.patch | 14.33 KB |
#1
a) Doesn't apply cleanly anymore (my fault, sorry it took so long to get to this)
b) this:
+ if (isset($releases) && count($releases)) {would be better as:
+ if (!empty($releases)) {c) this:
+ $output .= isset($group) ? $group : NULL;would be better as:
+ if (isset($group)) {+ $output .= $group;
+ }
d) same here:
- $output .= "$rss $link";+ $output .= (isset($rss) ? $rss : '') . ' ' .(isset($link) ? $link : '');
your version is hard to understand, and doesn't follow our coding conventions regarding spaces and string concatenation.
otherwise, this is a good start, thanks! we'll certainly want something like this before we start porting to D6, so we might as well get this fixed up and committed to all branches.
also, even though i already committed the other patch that included a bunch of similar code, I'm having second thoughts about this style:
+ '#default_value' => isset($query->priorities) ? $query->priorities : NULLI wonder if this would be better for both readability and speed (after the initial array definition):
if (isset($query->priorities)) {$form['priorities']['#default_value'] = $query->priorities;
}
I'm not sure which I prefer, but I wanted to raise it in case anyone else sees this issue and has strong opinions. ;)
#2
tried to apply these by hand to HEAD and then worked on cleaning up some others.
#3
applied with some minor adjustments to 5.x-1.x. doesn't apply at all to either 4.7 branch -- i'll leave it as to be ported if anybody is interested,,,
#4