>>>>> "SB" == Steve Bertrand <st...@ibctech.ca> writes:
SB> Uri Guttman wrote:
>>>>>>> "SB" == Steve Bertrand <st...@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.
SB> Thanks Uri,
SB> I get the gist of what you are doing here. My example 'item_name' was a
SB> bad one ;)
SB> 'item_name' is literally a key in a hash where the value is the name of
SB> a purchased item. It is extracted from a hash that has ~10 other
SB> purchase-type items (comment, amount etc) ;)
SB> Not every key in the incoming %$data parameter has the same value type,
SB> so AFAIK, I have to check each item in the hash individually as opposed
SB> to just assigning it to a temporary var. (...I'm working away from a
SB> predecessor's method of using $tmp, $a, $b, $c, $left, $right and
SB> everything else, so I don't do things like that ;)
SB> Common-code doesn't really bother me, so long as I'm writing it in a
SB> context where it needs to be repeated for different data types, and once
SB> it's done, I can re-use it. Shrinking the repeating code blocks is a
SB> different story.
SB> I'm learning new tricks far faster than I can code. If I can get 100
SB> lines of great working code done today, then while I'm reading tonight,
SB> I'll learn new ways on how to write that code ;)
SB> Thanks Uri,
SB> Steve
SB> ps. The module in question is here: http://ipv6canada.com/Sanity.pm
SB> pps. I'm currently reading "Advanced Perl Programming" by Sriram
SB> Srinivasan. (Yes, I know there's a new version).
SB> Steve
--
Uri Guttman ------ u...@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 ---------
--
To unsubscribe, e-mail: beginners-unsubscr...@perl.org
For additional commands, e-mail: beginners-h...@perl.org
http://learn.perl.org/
