>>>>> "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 ---------