21

I've generally been working with PHP warnings and notices off, since I work on a lot of projects where it's already in live production. Now, if I turn on the warnings and notices on these live production websites, they'll be overloaded with them.

The projects that I work on at home, on local, I usually try to work away ALL warnings and notices. Sometimes, there's no solution to not having a notice, so I'd just have to deal with looking at the notice until I decide to turn them off altogether.

In the end, I don't know whether I'm wasting my time trying to get rid of all warnings and notices, or that I'm actually doing this for the greater good.

Hence my question, is it good practice to avoid warnings and notices altogether, or does it really not matter?

iRcode
  • 3
  • 1
Audite Marlow
  • 379
  • 2
  • 7
  • 6
    "Sometimes, there's no solution to not having a notice" It's been a while since I used PHP, but I don't recall running into an cases where you could avoid the notice/warning or at least [suppress it locally with `@`](http://php.net/manual/en/language.operators.errorcontrol.php). – CodesInChaos Jan 21 '16 at 13:59
  • 27
    The most convincing argument I've seen is "those messages exist for a reason - conditioning ourselves to ignore a flood of warnings causes us to overlook actual problems that may have been averted." In other words, if there are no warnings or notices during *normal* operations, any warning or notice is a sign of potential trouble; if everything is just noise, you'll start noticing trouble only after SHTF (and probably after the customers do). – Piskvor left the building Jan 21 '16 at 14:02
  • 12
    Using @ to suppress notices, while common, is generally considered a bad thing. It's worse then simply turning off all notices because you have now hidden a potential problem. In 15 years of php programming I have yet to come across a case where I have had to suppress a notice in code I control. – Cerad Jan 21 '16 at 14:48
  • 2
    Are you simply turning off the display of these notices or are you doing `error_reporting(0);`? I always use `error_reporting(E_ALL);` and the only difference between development and production is `ini_set('display_errors', 'on');` vs `ini_set('display_errors', 'off');`. I always aim to fix notices and warnings while the code is still fresh in my mind. I frequent the logs on my production system to see if there are additional warnings and notices which I may have missed. – MonkeyZeus Jan 21 '16 at 16:00
  • Not for the greater good, just for your own. – Carsten S Jan 21 '16 at 18:15
  • 1
    I very much agree with what @Cerad said about `@`. After years and years of PHP programming, I haven't used that operator. Not once. Never. Not only does it hide potential problems, it also has a performance impact: behind the scenes, PHP switches off error reporting before calling the code -> calls the code -> sets it back to its original value. These steps are expensive if you have dozens or hundreds of `@` in your code. – Radu Murzea Jan 21 '16 at 21:53
  • @RaduMurzea `json_decode`, and sometimes `file_*` functions occasional need to use the `@` because otherwise a malformed json string could show a notice. But you are right there is a performance penalty as well. – AKS Jan 22 '16 at 00:52
  • If you can't find a way to avoid a notice or warning, switching it off with @ is ***much*** better than turning off all warnings. Yes you're "hiding a potential problem", but turning off all warnings is like putting @ on every single line! – Ben Jan 22 '16 at 05:52
  • @Cerad Using `@` is perfectly fine if you check the return value for errors and handle them appropriately. Not every API lets you rule out the possibility of errors before calling it. For example how would you convert user supplied (and thus potentially invalid) data between charsets if not by using `@iconv` and checking the result for `FALSE`? | The new `random_int` API would have almost ended like that before it was changed to raise an exception instead of emitting a warning and returning `FALSE` when it fails. (I am very happy they made that change) – CodesInChaos Jan 27 '16 at 16:53

3 Answers3

49

If warnings and notices are coming from your code, most definitely fix it. From my experience, in 95% it may be benign, but the 5% highlight a real problem that may lead to countless hours spent chasing.

If they are coming from the third-party code that you must use for one reason or the other, you generally don't have much choice.

It's a different question if your legacy codebase is really large, then you can treat legacy code as third-party, but require the new code be warning-free.

Alexander Pogrebnyak
  • 566
  • 1
  • 5
  • 6
  • 8
    I work in Java/eclipse, which is different than php obviously, but I usually find the warning is raised by either 1) something that compiles but I made an obvious mistake or 2) something that is fine now but will be bad down the road – corsiKa Jan 21 '16 at 16:19
  • 1
    @corsiKa I *was* translating your comment to PHP but I realised that only one word needed to be changed. – wizzwizz4 Jan 21 '16 at 19:08
  • 3
    Unless, of course, these warnings are from StyleCop about your order of your `using` statements... – Dan Jan 21 '16 at 22:59
26

if I turn on the warnings and notices on these live production websites, they'll be overloaded with them.

You should always have warnings turned on to the fullest level in development, testing, and QA, but not in production. Actually, if it's a dogfooding application, i.e. an application that you use yourself, then you should also have them turned on in production.

Basically: have them turned on in those cases where the person who sees them is in a position to do something about them (the developer in development and testing can fix them himself, the tester in QA can file a bug, and if the developer is also the user, then he can also fix it in production), but don't turn them on when the person who sees can't do anything about them (a user in production, who doesn't even know how to program).

Ideally, you'll also want to turn on treating warnings as errors, but that only works if there are none to begin with ;-) But keep this in mind as a goal! If it is possible to turn this on/off on a per-file basis, turn it on for all new files, and turn it on for all warning-free files, and never turn it off again once turned on.

So, what to do about the overload?

You make a list of every warning and notice, and then adhere to the following rules:

  1. Never, ever, under no circumstances add a new warning to the list. Every new piece of code, every edit, every change, every patch, every commit must not introduce new warnings, it may only ever fix them.
  2. Everytime you touch a piece of code, fix any and all warnings in that piece of code. (The Boyscout Rule: always leave the campsite in better condition than you found it.) That way, the un-important code can stay full of warnings, but the important code will get cleaner over time. "Piece of code" may be a function, a class, a file. You may also relax this rule to say to fix at least one warning. The point is: fix them as you find them.

Note: both of these require that you have some sort of log database and log filtering mechanism in place. Note also, that "log database" and "log filtering mechanism" could just be a text file and grep.

This is the important bit. Without the database, you won't know when you add a new warning, and without the filtering, you still have the overload problem.

Note #2: this doesn't only work for warnings, it also works for style checkers, complexity metrics, code coverage, static analysis tools, and so on. Basically:

  1. Don't add new problems.
  2. Fix old problems as you stumble across them.

This allows you to easily prioritize: code that is edited often and thus needs to be easy to read and maintain, will get better over time. Code that isn't touched often, won't get better, but that's okay, because nobody needs to look at it anyway. And, at least it won't get worse.

Of course, nothing stops you from allocating time specifically in order to do nothing but hunt down and kill warnings. It's just that often, this isn't economically viable, and it is your job as an engineer to keep that in mind. "An engineer is one who can build with a dollar, what any fool can build with two."

Jörg W Mittag
  • 101,921
  • 24
  • 218
  • 318
  • 3
    Another point why to turn off warnings and errors that reach the user unfiltered: As informative as a warning is to the developer, it might leak sensitive info (names of files, names of other involved servers, structure of sql queries used, ...) – Hagen von Eitzen Jan 21 '16 at 20:30
  • Warnings in production should go to the logs, not to the user! *Errors* should go to the logs, not to the user. A website that doesn't trap errors, log them, and serve up a user-suitable error page instead isn't production ready. PHP makes it very easy to do this wrong, but you should still do it right. – hobbs Jan 22 '16 at 04:51
12

It matters. A warning might not break your tests or even show up in the wild for a while -- but it could be a symptom of a looming bug. I currently develop primarily in C#/C++ and have a defined strategy to get rid of and keep warnings out of our code base. Luckily it's not rocket science =).

If the language your working in has the ability to treat warnings as errors and has variable warning levels, I would do the following:

  1. Turn the warning level down just far enough so you don't get any warnings. If you are at the lowest warning level and you still get warnings -- try to fix them. If you can't fix them, then you're done for now, but hopefully you can fix them. Great.
  2. Since you now have no warnings (at a low warning level most likely), flip the switch and treat all warnings as errors.
  3. Try to turn the warning level up and fix all the new warnings. If you can't, turn the warning level back down, but don't turn off treat warnings as errors.

I find that this not only works warnings out of my code -- it keeps them out.

PerryC
  • 271
  • 1
  • 8