[Israel.pm] FW: defined function

bc.other bc.other at gmail.com
Sun Jan 18 02:33:54 PST 2009


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: perl-bounces at perl.org.il [mailto:perl-bounces at perl.org.il] 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
Perl at perl.org.il
http://perl.org.il/mailman/listinfo/perl




More information about the Perl mailing list