Managing error reporting level. We could use cleaner code.

QuestionsManaging error reporting level. We could use cleaner code.
Doug asked 9 years ago

We use phpgrid in a complex application. Some bugs in our code are hidden because phpgrid turns off E_NOTICE reporting. We can turn this back on by modifying the first line of code, but this is not practical because phpgrid generates so many notification messages.

The number of notifications make us question the quality of phpgrid. The code has inconsistent use of isset() and undeclared variables. The comments in the code suggest that many notifications were introduced by someone "fixing" the code with poor style.

A new version that did not cause so many notifications would be great. Is that possible? I started fixing the code but realized that this would prevent us using new versions.

Thanks in anticipation.

5 Answers
Abu Ghufran answered 9 years ago

Hello Doug,

PHP Grid was developed with E_NOTICE turned off, so yes it will have many notices.

For your case, solution is to replace the first line of jqgrid_dist.php with

$err_flag = error_reporting(E_ALL & ~E_NOTICE & ~E_DEPRECATED);

This will preserve your last error reporting flag. And at the last line of jqgrid_dist.php reset it back to your flag.

// reset last error flag
error_reporting($err_flag);

….

Regarding product quality, Our developer community is using and reporting issues since 2011. So we are constantly fixing & optimizing it and this has significantly improved the reliability factor. And i think, at the end of the day, unless you are extending the library code, it's code reliability that matters end user not the code quality.

And yes, we admit it's a code quality issue and should be fixed but it may take (fixing + testing) time.

Abu Ghufran answered 9 years ago

Ok, I'll be working on it and update you after some resolution.

Doug answered 9 years ago

Abu,
Thanks for the prompt reply. I have added the code you suggested. It helps but the code in custom handlers still has the problem so I followed the same pattern of resetting the error_reporting level in your code before invoking custom handlers (on_add, on_edit and on_delete). With those additional changes I am happy.

Please consider making these minor changes to a next release.
Doug

Doug answered 9 years ago

Abu, after further use of the modified grid I don't think your suggestion works. With your approach the notifications from the initial include are suppressed but each time the methods of the grid are invoked notifications are still generated. I added the error reporting set/reset code into the the constructor, and the methods render_js(), set_options(), set_columns() and render(). This makes the grid "quiet" but still does not fully fix the problem. The method render() alone has 9 exit points (die calls), so the reset code does not get called at the end of the method so notifications in our code would still be missed.

I am not sure if I am missing something obvious, but I don't think this is a practical fix.

Abu Ghufran answered 9 years ago

Hello Doug,

Although i was unable to remove all notices from jqgrid_dist.php, i just found an alternate solution for your case.

This code will connect custom error handler and ignore if the notice are generated by jqgrid_dist.php otherwise do the normal error handling procedure. Place this code at start of your app code.

http://hastebin.com/metazodipa.php

You can simply remove / comment top 2 lines from jqgrid_dist.php
// error_reporting(E_ALL & ~E_NOTICE & ~E_DEPRECATED);
// ini_set("display_errors","off");

This is minimum risk solution so far by my research.

Your Answer

1 + 0 =

Login with your Social Id:

OR, enter

Attach code here and paste link in question.
Attach screenshot here and paste link in question.



How useful was this discussion?

Click on a star to rate it!

Average rating 0 / 5. Vote count: 0

No votes so far! Be the first to rate it.

We are sorry that this post was not useful for you!

Let us improve this post!

Tell us how we can improve this post?