Hundreds of Notices and Warnings in PHP code

Question

I've inherited some legacy code, within which if I turn on:

error_reporting(E_ALL); 

I get literally hundreds of messages all over our site, all of them like:

Warning: include_once() [function.include]: Failed opening '../inc/variables.php' for inclusion (include_path='.:/usr/local/lib/php') in /var/www/html/xyz/xyz/xyz/payment.class.php on line 9

Notice: Undefined index: validateArrayName in /var/www/html/xyz/xyz/xyz/payment.class.php on line 3417

Notice: Undefined variable: objInstructor in /var/www/html/xyz/xyz/xyz/classes/metatags_class.php on line 44

Is this a bad thing? The site works fine otherwise, but I'm wondering if its

(a) a really bad thing and 
(b) even if not, is it worthwhile to go fix all these issues?

Answer 1

The site works fine otherwise

Hmm, the failing include_once() does not ruin anything? In that case (i.e., you're not using anything - variables, globals, functions - from ../inc/variables.php) just remove the line.

You can probably ignore notices ("ugly" code), but should keep an eye on warnings (missing functionality). For legacy code always balance effort and benefit. If it works (and you're willing to accept potential security breaches), let it run ... but phase it out over time. If you're not that confident, rewrite.

Answer 2

Try finding where the file ../inc/variables.php is present (it is not present at /var/www/html/xyz/xyz/xyz/inc/variables.php where it is supposed to be), since the file /var/www/html/xyz/xyz/xyz/payment.class.php is trying to include it. I have a hunch that this file contains the definitions (initializations) of all the variables you are seeing in the warnings.

If after including the correct variables.php file, you get the same 'Undefined variable' warnings, in my opinion there is no need to go and modify 'legacy code' to define all the variables.

See how it goes.

Answer 3

Are you sure your site works fine? Do you have a good unittest suite that's run regularly to make ABSOLUTELY sure?

Failed file includes are almost always bugs (or truly horrible coding practice). If your set ever does unexplained or weird things, these problems are likely the cause. If it really has so many problems, are you sure it wouldn't be faster to rewrite it in a clean, consistent, careful style?

In general, yes you should fix these problems if you intend to continue developing this code base. If it's an orphan project or you expect to rewrite from the ground up, don't bother.

Answer 4

There is lots of things you cannot test from the UI of an application. Stuff that happens under the hood. Unless your application is fully Unit-Tested or Functional Tested, you are likely not seeing the whole picture by clicking yourself through it.

You should definitely look into the Warnings. The developer probably didnt include variables.php for fun, so you should double check that payment.php works as expected.

Notices are less severe but they are still indicators of sloppy code. Usually, they are not that hard to fix, so unless you are on a very tight budget, fix them.

You also might want to change error_reporting from E_ALL to -1. That will enable all erros plus E_STRICT errors and anything that might be added to PHP in later versions.