Undefined index everywhere

[henesnarfel]

Just pulled the latest master. I know this isn't fully released yet but was trying to debug something and I put the below in the ost-config.php

error_reporting(E_ALL);
ini_set('display_errors',1);
ini_set('display_startup_errors',1);

Undefined index everywhere. Hopefully this won't be left like this when finally released.

[protich]

You should see error reporting settings in main.inc.php - Notices are suppressed by default.

PS: Display errors will be set to off on stable release and even RC releases going forward. But I doubt we'll refactor the code to define every variable system wide.

[henesnarfel]

Didn't see those settings but I do now.

As for not fixing the errors. In my opinion that is unacceptable coding practice. Especially for something that is going to be used for a pay service in the future. To me (and what I've seen when searching) an error is an error no matter how small and should be fixed. It can also cause a performance hit as well depending on the traffic the server gets and the amount of errors. Also depending on the error log settings for the server it could fill up the logs very quickly. If I'm trying to build out a new feature that I would like and I'm trying to debug it, it is almost impossible to see what may be wrong with my code. Sorry if this seems rude or offending but this is how I feel about this issue.

http://stackoverflow.com/questions/4261133/php-notice-undefined-variable-and-notice-undefined-index http://stackoverflow.com/questions/5343811/do-too-many-undefined-index-or-undefined-variable-errors-effect-server-performan http://stackoverflow.com/questions/1868874/does-php-run-faster-without-warnings

The image below shows what is displayed if errors are turned on on the default admin panel page.

[protich]

I understand where you're coming from - I've done plenty of C development where undefined variable is an error. But this is PHP - not being able to define variable is allowed (considered a feature by some). Download any PHP application and I can almost guarantee you - you'll get plenty of notices due to undefined variables (I challenge you to do same on your Moodle fork and post the results).

osTicket error reporting setting excludes notices by default. In your case, I don't see how useful notices are in debugging your code. I can see the need to turn on error display on - but notices?

PS: If I was to put on my purist-coder hat - I won't develop an application in PHP.

[henesnarfel]

Two widely used open-source php applications Codeigniter and Moodle and neither display undefined variables unless there is one.

Moodle Before

Moodle After

Codeigniter Before

Codeigniter After

[protich]

I stand corrected on Moodle - but note that osTicket issue is undefined indexes - not variables.

We go through great pain to "save" what the user enters on the forms on error, and also displaying the corresponding error message - it's the main reason for many of the undefined indexes. The issue will be addressed when we move to a templating engine in v1.8.

PS: For 1.7 code base I'll consider refactoring the code (post 1.7 release) to use a getter - which will check if the index is set/defined i.e $ost->get_var(...);

function get_var($index, $vars, $default='', $type=null) {
    if(is_array($vars) 
           && isset($vars[$index]) 
           && (!$type || gettype($vars[$index])==$type))
        return $vars[$index];

    return $default;
}

[henesnarfel]

The getter is a good idea and would satisfy my OCD.