2

Should I use E_NOTICE enabled on live server?

As I can see in Why should I fix E_NOTICE errors? everybody is again with using isset() to omit notices. I have read pros, but how about code readability?

Let's examine some code:

if ($this->_oldattributes['category_id'] != $tattr['category_id']
    || $this->_oldattributes['deal_id'] != $tattr['deal_id']
) {

Clear to understand what I want here, right?

Lets add isset() to omit notices:

$old_cat_id = isset($this->_oldattributes['category_id']) ? $this->_oldattributes['category_id'] : null;
$new_cat_id = isset($tattr['category_id']) ? $tattr['category_id'] : null;
$old_deal_id = isset($this->_oldattributes['deal_id']) ? $this->_oldattributes['deal_id'] : null;
$new_deal_id = isset($tattr['deal_id']) ? $tattr['deal_id'] : null;
if ($old_cat_id != $new_cat_id || $old_deal_id != $new_deal_id) {

Not bad, but now I need more time to understand what is happening here.

A few bits about debugging - I never wasted a day to debug errors caused by not initialized variable. Yes, maybe half of a hour was wasted to slide down into the classes/functions but this not a big waste when comparing to losing code readability IMHO?

A few bits about performance - http://seanmonstar.com/post/909029460/php-error-suppression-performance

The difference was that suppressing the notice took 100% as long as just checking if it existed first" Test was performed on 1000000 supressions. How many of them appears in your code, even if you use Zend+Doctrine+something else? And how many time does it takes in comparison to DB connections, etc.?

Maybe I just not used to write correct code(used only PHP for long time now).

For context we have moved our project to server which displaying E_NOTICEs, that's why I'm asking this. I don't know why this is a big problem to switch them off?

Another example:

class Item extends CActiveRecord {
    private $_oldattributes = array();

public function afterFind()
{
    // Save old values
    $this->_oldattributes = $this->getAttributes();
    $this->_oldcategory = $this->category;
    $this->_olddeal = $this->deal;
}

so I will have null if old attributes not exists and:

$tattr = $this->getAttributes();
$this->_oldattributes['category_id'] != $tattr['category_id']

which will give me what I need, and there are no errors, right?

Same with not an object error, in Yii when I have set up relations some objects may not be initialized, yes there is a kind of error, because have null value and not initialized in this case is real error sometimes:

$this->deal->item->id != $this->_olddeal->item->id

This may trigger a notice but will work as intended, so you ignore the notice:

if ($_GET['foo']) ...

The problem is that I can't ignore this, because it still throws notice and every time when i use this I must write:

if (isset($_GET['foo']) && $_GET['foo']) ...

or:

if (@$_GET['foo']) ...
Community
  • 1
  • 1
llamerr
  • 2,997
  • 4
  • 29
  • 40
  • Yes, this is happened already and I need to fix all my notices now :) but I don't understand why I can't switch it off? – llamerr Apr 09 '12 at 17:36
  • Sorry edited original comment – llamerr Apr 09 '12 at 17:37
  • 4
    You should fix E_NOTICE *errors* if they are in fact errors. (E.g. bare words instead of constants. Undeclared vars are handled quite well by the Zend runtime, as that's its raison d'etre). Supressing notices with the `isset()` syntax hump is no more clever than using `@()` the language builtin. Even less sensible if the whole reasoning is [microoptimization](http://qkme.me/35ksv1). – mario Apr 09 '12 at 17:38
  • 1
    You should post this as an asnwer, @mario. – sshow Apr 09 '12 at 17:44
  • But they are still displayed, even if it's not errors, can I omit undeclared variables, undefined offsets, i.e. specify more what I want to display and what not? – llamerr Apr 09 '12 at 17:45
  • 1
    One reason I always leave notices on: To catch spelling mistakes in variable names. If I set/declare `$companion` and try to use it later with `$companian`(sic) PHP will politely let me know with an E_NOTICE. – Mike B Apr 09 '12 at 18:00
  • 1
    Isn't this essentially a duplicate of the question that you linked to? `:-|`. The case for fixing all notices and warnings is very well described on that page, but some codebases are so bad that fixing it all may not be feasible. If this applies to you, do an hour or two of notice-squashing per week - it's good dev practice anyway, and helps detect/reduce bugs. – halfer Apr 09 '12 at 18:04
  • Yes, this is useful. I use variable highlighting for this. If my variable not highlighted with others there is something wrong with it – llamerr Apr 09 '12 at 18:05
  • I asked this because old question was asked a year ago, I hope if I ask it in new question I will receive more answers. check last edit too, please – llamerr Apr 09 '12 at 18:14
  • 1
    @llamerr: The question you link to hasn't yet been closed. What makes you think it won't receive new answers? – BoltClock Apr 09 '12 at 18:22
  • tried to ask and yes, you were right. didn't think that community is so active, so just duplicate question, my bad – llamerr Apr 10 '12 at 16:16
  • "Should I use E_NOTICE enabled on live server?" - no, it's a security risk. Notices can be used to determine locations of files or database structures, and may help crackers find a vulnerability. – halfer Apr 11 '12 at 11:44

2 Answers2

3

When I look at your code, for example

$this->_oldattributes['category_id']

my first thought was, that this should probably be a class instead. 

class Attributes {
  public $categoryId;
  // and so on
}

The difference now is, that don't need to use isset() anymore, because now the property exists (maybe it's null) or it is definetely a bug ("Undefined property") and should be fixed. I think most of your cases, that you "fixed" with isset(), are of this kind.

KingCrunch
  • 128,817
  • 21
  • 151
  • 173
  • seems my problem was in enabled E_NOTICE on live server, so I think I will accept this answer and search for this question instead, thanks – llamerr Apr 09 '12 at 18:22
1

When debugging the code it is nice to have undefined variable messages when variables that should be defined is not defined. I think it is good practice to define your variables before use, as far as i know other languages are more strict on this than PHP.

datagutten
  • 163
  • 1
  • 9