Hi, Thanks for your remarks, this sub is used internally by me, and not by others. This is why I checked the values against 1 and 0 and not to any "other value"
The idea was: RaiseError will cause the program to exit using the exit code, PrintError will cause the print of the message into STDOUT This way I can use it to just send a warnings, which I should have changed the "Error:" inot "Warn:" (in the print out) Or just use it to send Error message, and allow the script to continue / or not - depends on the user decisions The default values, says print the error and exit using exit code. As for sending the exist code any other time then numeric value - is not an issue - since I know for sure the user will Want a numeric answer coming back. Thanks Chanan -----Original Message----- From: [email protected] [mailto:[email protected]] On Behalf Of sawyer x Sent: Sunday, January 18, 2009 12:08 PM To: Perl in Israel Subject: Re: [Israel.pm] FW: defined function - Obviously, you should use "defined" instead of not at all. That's an easy question. - Also, with hashes you should use "exists" to check if there is such a key. Otherwise, you would get a warning, because it's tried to access a value of a key that doesn't exist. that's obviously a problem, don't you think? :) - The reason why use strict && use warnings did not prevent the script from running it is because it's a runtime error, and not compile time. strict (or warnings) would suffice with warning you while running it (during runtime). You should be able to use judgment, "exists" and "defined" to avoid such problems. - when sending a hash to a function, it's better - for health reasons - to send a hash reference instead. To create an anonymous hash reference, use {}. Such as { printError => 0 }. I had only a few comments on the subroutine you pasted: - I would write it a bit cleaner: no if()s at the end, using "exists" for hash values, etc. - Wouldn't you want RaiseError to print the error as well? Or just exit? - Simple design problem: You take into account that perhaps someone wouldn't send an exit error, but then if they send a hash, it will be create havoc with the parameters in the subroutine. That's a good reason why you would use references. That way, if a person does: printWarningError("msg", { RaiseError => 1 }), it will pass the hash reference to $lv_ec. However, you'll have to check if $lv_ec is actually a hash reference instead of an exit status. So, instead, I would suggest to put the exist status in a hash. You could also put the error msg in the hash, but you probably will definitely have a msg, so you don't have to put it in the hash. That way, it would look like this: printWarningError( "my msg", { RaiseError => 1, ExitStatus => 0 } ); The way I would revise your subroutine, with your permission: sub printWarningError { my ( $lv_msg, $lv_args_href ) = @_; # message to print $lv_msg || return 0; # defualt values if ( ! exists $lv_args_href{'ExitStatus'} ) { $lv_args_href{'ExitStatus'} = 0; } foreach my $arg ( qw( PrintError RaiseError ) ) { if ( ! exists $lv_args_href{$arg} ) { $lv_args_href{$arg} = 1; } } # this is actually better than writing # if ( !exists $lv_args_href{'PrintError'} ) { $lv_args_href{'PrintError'} = 1 } # if ( !exists $lv_args_href{'RaiseError'} ) { $lv_args_href{'RaiseError'} = 1 } # # because it provides less duplication, and makes it easier to make changes chomp $lv_msg; if ( $lv_args_href{'PrintError'} ) { print "Error: $lv_msg\n"; } if ( $lv_args_href{'RaiseError'} ) { exit $lv_args_href{'ExitStatus'}; } return $lv_args_href{'ExitStatus'}; } printWarningError( "my msg", { PrintError => 0, ExitStatus => 0 } ); ---- It seems longer (of course, notice I put a whole block of comments to explain why I used 5 lines - include end statements - instead of 2), but it's much more expressive, and allows to expand your code much more. It's also more dynamic and checks better whether something is defined or not. _______________________________________________ Perl mailing list [email protected] http://perl.org.il/mailman/listinfo/perl _______________________________________________ Perl mailing list [email protected] http://perl.org.il/mailman/listinfo/perl
