>>>>> "SB" == Steve Bertrand <steve@ibctech.ca> writes:
SB> The problem I have, is that I don't like the fact that the "if"
SB> condition contains the exact same line of code that a sub-section of the
SB> add_message() function is receiving as a parameter. I know there is a
SB> way to bundle it better, but in my testing, I haven't been able to do it.
use some temp vars (NOT named temp) to factor out the common code.
my $item_name = $data->{item_name} ;
SB> if (length($data->{item_name}) == 0) {
unless ( length( $item_name ) ) {
length will be false if it is 0 so you don't need to check against 0.
and if you don't allow '' or '0' you can drop length as well.
SB> $error->add_message( "item_name is undefined" );
SB> }
SB> if ($self->safe_string($data->{item_name})) {
if( my $safe_name = $self->safe_string($item_name) ) {
SB> $error->add_message( "item_name has potentially dangerous chars:".
SB> $self->safe_string($data->{item_name})
$error->add_message(
"item_name has potentially dangerous chars: $safe_name" )
you can also use statement modifiers for the if blocks now that the code
is shorter
so it looks like this now:
my $item_name = $data->{item_name} ;
$error->add_message( "item_name is undefined" )
unless length $item_name ;
my $safe_name = $self->safe_string($item_name) ) {
$error->add_message(
"item_name has potentially dangerous chars: $safe_name" )
if $safe_name ;
that is shorter, faster (no blocks, no duplicate code), and easier to
read in general.
uri
--
Uri Guttman ------ uri@stemsystems.com -------- http://www.sysarch.com --
----- Perl Code Review , Architecture, Development, Training, Support ------
--------- Free Perl Training --- http://perlhunter.com/college.html ---------
--------- Gourmet Hot Cocoa Mix ---- http://bestfriendscocoa.com ---------
|