Well, there was an interesting side effect of parsing my web logs to identify all the places where I'm throwing warnings.
I've got a relative count of how many times the code is being called. Of course, it is only the code that causes warnings that is being counted...
Anyways, this is my most troublesome snippet, accounting for 80 percent of the current warnings.
$form .= $args->{'hidden'};
$form .= '';
$form .= $args->{'hidden'} if $args->{'hidden'};
as$form.= $args->{'hidden'} if $args->{'hidden'};
If the value of $args{'hidden'} evaluates to false, "" is used instead, and the undef value isn't used for concatenation.$form.= $args{'hidden'} || "";
Re: cheap profiler
phillup on 2004-08-13T23:19:44
But note that 0 evaluates to false, so if you expect that as a legit value of $args{'hidden'}, this won't work.
Hm... now that you mention it, mine won't either if the string is '0'.
It isn't a problem in this specific instance... but I may need to actually use 'exists' on the value as a more 'generic' solution.
Re: cheap profiler
jplindstrom on 2004-08-14T12:23:29
You could always create a small utility function to keep things nice and tidy.or something like that.sub definedOrEmpty {
defined($_[0]) ? $_[0] : "";
}Maybe a shorter name would be better. orEmpty perhaps. Whatever's clear in your situation.$form.= definedOrEmtpy( $args->{'hidden'} );
Also, 100% of the errors have to do with string concatenation, or comparison operators where I expect that values may be 'undef'.
Classic mistake: Some of my code expects undef values so I ignore uninit warnings.
Problem is the rest of your code doesn't expect undef warnings, in those spots getting an undef indicates a possible mistake. By ignoring uninit warnings for one part of the code you ignore problems in the rest. If a certain part of your code expects undefs write it so it handles them quietly (more below).
So... I'm about to spend a lot of time eleminating warnings that are "harmless". Because, even with all my bitching... I know that there will be a payoff... eventually.
Classic mistake: This warning isn't really indicating a problem so I'll ignore it.
Folks have found that one broken window can lead to an entire building being trashed. Minor damage leads to a sense of abandonment and people figure if one window is broken nobody will care if another one is and so on. A smaller scale version of this is your kitchen sink. Think about how much easier it is to leave a pile of dirty dishes in a sink if the kitchen is already a little messy.
Once you have one "expected" warning you'll have two. Then three, four... until you can no longer tell the expected warnings from the unexpected ones and warnings are completely ignored. Don't start down that road.
Enough theory. Here's a bunch of ways to handle your problem.
The various ways already talked about to check for definedness at the point where you use the data. Problem is you have to sprinkle these checks all over the place which gets tedious and is prone to mistakes. This we want to avoid.
Initialize $args->{hidden}. Its generally easier to initialize values you expect to be blank than play with expected undefs. You only have to do it once at the start rather than every time you use it.
Put it in a function. If you're literally writing
you're repeating yourself. Something as basic as$form.= $args->{hidden}
will eliminate the duplicate code. Then, since you're only writing it once, its less bother to account for possible undefs.add_hidden_fields($form, $args->{hidden});
Use a module. If a lot of what you're doing is creating forms there's a whole bunch of modules out there to do it for you. You might not think its worth while, after all you're just concatinating a bunch of simple HTML. But complexity creeps in, like undef warnings, and you'll find ad hoc string concatination rapidly becomes inadequate.
Tie it. In some cases, such as HTML form input, there is a possible set of keys you expect in a hash but they won't always be there and its ok if they're not. So you represent your form input with a tied hash. Any attempt to fetch a valid key that has no data results in '', but using exists() on the same key returns false. A simpler solution is to use a tied hash which returns '' instead of undef for anything which does not exist. Not as good since it hides typos but a good place to start.
Re:No such thing as a harmless warning.
phillup on 2004-09-29T20:32:32
Classic mistake: Some of my code expects undef values so I ignore uninit warnings.
Actually, I am not ignoring any warnings. I added the use warnings pragma and it did what it was supposed to do... flagged possible problems.
Then, I looked at each and every case and saw that they were all the same class of errors (concatenation of undefined variables or comparison with undefined data) and inspected each and every one and determined that none of them was a bug causing issue.
Classic mistake: This warning isn't really indicating a problem so I'll ignore it.
Then, for each and every one of the warnings that I inspected and determined not to be a problem... I corrected the code so that it would not issue a warning.
I did not ignore it.
Instead, I read every single thing written by you and Andy that I could find... and took it all to heart.
Which is/was part of the problem. I could not go back in time and start writing all of this code the "correct" way. So, I have to retrofit testing onto the existing code.
As I write the tests and find bugs, including warnings, I fix them.
But, the code must stay in production. And, warnings are not emmitted until the code is actually run. So, there is code running that I have not (still!) got tests written for.
When that code runs, the warnings are emitted and placed in the log file of the server. So, every day I've been monitoring those files for warnings from code that is not yet covered by a test script.
Then, I inspect and fix the code so that no warning is generated.
My point of the post was that starting the process of writing test code, and the use of the 'warnings' pragma was causing the warnings in the log file *because* my test do not yet cover all of the code. (IOW, the warnings come from code not yet covered by the test suite.)
And, the point was, that no matter the source of the warnings... just because they did not show in my test suite... they still should be addressed.
Not ignored.
This post was actually a follow-up to a previous post where I tried to explain that the errors should not (and would not) be ignored. It probably doesn't stand well on its own... hard to get context.
Put it in a function.
I found it a bit interesting that you advocate replacing concatenation with a function. While I would only have to change the one function (if there was an error), I don't see how it would eleminate duplication. I would simply be replacing the use of the dot operator with a function call. (And, the function would end up using the dot operator anyway.)
...and you'll find ad hoc string concatination rapidly becomes inadequate.
Actually, that is why I didn't use a module. Because I needed more flexibility (ad hoc) than most of the modules I've seen offer. (Remember, I started this over two years ago.)
BTW... this is in a Mason environment. With that in mind, I'm really open to suggestions if you have a module in mind for me to look at.
Advice is always welcome here!
A simpler solution is to use a tied hash which returns '' instead of undef for anything which does not exist.
Hm... that is a very intersting thought... thanks!