PHP Notice messages in Vanilla

gugglegum

I believe that good PHP scripts does not generate any PHP errors (including notices). Otherwise, if PHP message is occured -- this is always means that PHP script is not good enough. There is a set of PHP errors with E_NOTICE level in Vanilla base scripts. I found it's a bad and dirty coding style when for proper execution I have to disable errors output via "display_errors 0" or via restrictions from error_reporting levels. There is not so much notices and I've fixed most of it within 15 minutes just by adding several "if" conditions to check existence of objcets that can be missed. For example, $this->Discussion is not defined in library/Vanilla/Vanilla.Control.CommentGrid.php when user access non-existing/removed discussion, but there is no check in section called "// Set up the pagelist". So my questions is: why Vanilla is not tested with error_reporting = E_ALL ?

y2kbg

uhh I get all notices when they occur, but they hardly ever do, only when I am testing an extension that I am working on. Maybe you messed with something?

Vincent

Vanilla by itself without any modifications doesnt throw up any errors if a proper installation has been run. If someone has a improperly coded extension then errors may occur, but that's only within extensions or anything that has been fiddled around with in the core framework. Otherwise, as i said before, nothing should throw up any errors.

gugglegum

I don't think so. Notices was in comment.php. For example, try to access discussion with non-existing ID. You'll get notices.

jeremy2

Notices are not errors - they are simply there to aid in debugging. It is generally not a good idea to report "errors" of E_NOTICE on a production web site, because it does not look good, and it is totally unnecessary because PHP will still process the request as the variable being the equivalent of NULL or 0 depending on the context - which is what the script designer usually intended in the first place. Therefore it's not a "dirty" coding practice. It is just as dirty as retrieving a number from a MySQL database and doing arithmetic on it: all data retrieved from the database is actually a string, and converted to numbers (if numeric) when you perform mathematical equations. Are you going to apply intval and floatval to all of your retrieved database information, too?

y2kbg

I would have to disagree, I always have all error reporting on because I like to know when my code isnt as good as it could be, also if i release an extension the leaves notices am I supposed to tell people to change their error reporting to make for my lack of adding a single line of code? I have also noticed notices help me trace a fatal error.

gugglegum

I would have to disagree too. You are right, notices are not errors, but normally they should not be. Notices is often are hints of real errors. For example, reading and comparing non-existing variable or object property is not an error, but this most likely it's a misprint of name of variable/property. Or constant that's not defined is automatically converted to string with its name -- not error too, but it's most likely misprint in its name or wrong order of include/require.

jeremy2

For distribution I always try to make sure that my code will not throw those notices as there is always some person out there with error_reporting set to E_ALL on their hosting provider. If I know that a variable might not be set, I prefer to preface it with @ instead of the usual if(isset($var_name)) { } stuff. If for my own stuff, then I usually don't care a whole lot as I know the code will perform the same regardless of whether the variable has been set or not.

jeremy2

gugglegum: I hate that PHP converts undefined constants into strings. I would much rather have the script die than to have this error occur. This is one instance where I feel you are right about the notices. The behavior of the script changes in this case, which is a really, really bad thing.

gugglegum

Object properties should be described in the class with proper security status (public, protected, private). Assigning new public properties "on the fly" is a dirty style. So if you write code clearly -- notice of undefined property is always relates to error in your code. @-syntax suppress not only notices, but warnings and fatal errors. If there will be a fatal error -- user (developer) will not see anything, just blank screen. Therefore I prefer to use if(isset($var_name)) { }. I use @-syntax only for function calls which may generate warnings that I handle. Good example of using @-syntax is: if (!$this->link = @mysql_connect($this->host, $this->user, $this->pass)) throw new DBAL_Exception(mysql_error(), mysql_errno());

jeremy2

I only use it for things like "if (is_array(@$some_var))" or "$some_var = intval(@$_GET['some_val']);" - basically only in times where having a null or 0 or empty string is perfectly acceptable and is checked for later in some way. I don't use it to suppress whole lines, only variables. Of course, if you are stuck writing scripts for PHP4 you really do not have the option of writing classes with the status properties. I do think there are proper times to create objects on the fly, though. I use JSON to transmit information to JavaScript, and in one particular instance I pass information about a dynamic menu structure to JavaScript. I suppose I could have used a named index array instead, but for all intents and purposes it looks the same when parsed through JSON. It seems that we have a lot of opinionated people on this forum. I like this: it generates interesting discussions.