Re: About BLOCK variables

I'll admit that it took me a while to figure out what these constructions were doing

$uid = ${$entry->get('uid')}[0] if defined $entry->get('uid');
# and
$uids{$uid}{$_} = ${$entry->get($_)}[0] if defined $entry->get($_);
[download]

I think using a temporary var to hold the array reference is preferable to calling the function twice. It also simplifies and clarifies (IMO) the code.

for my $entry ( $searchresult->entries ) {
    my $aref = $entry->get( 'uid' );
    my $uid = $aref->[0] if $aref;

    if( $uid ) {
        for ( @vars ) {
            $aref = $entry->get( $_ );
            $uids{ $uid }{ $_ } = $aref->[0] if defined $aref;
        }
    } 
    else {
        print "*** No uid for the following  entry:\n";
        print Dumper( \$entry );
    }
}
[download]

I have also come to prefer a little more whitespace inside parens, curlies and the like, but that a very personal thing.

Examine what is said, not who speaks.

"Efficiency is intelligent laziness." -David Dunham
"When I'm working on a problem, I never think about beauty. I think only how to solve the problem. But when I have finished, if the solution is not beautiful, I know it is wrong." -Richard Buckminster Fuller

Comment on Re: About BLOCK variables Select or Download Code

Replies are listed 'Best First'.
Re: Re: About BLOCK variables by Anonymous Monk on May 23, 2003 at 23:37 UTC
Yes, I was having problems trying to figure out exactly what `${$entry->get($_)}[0]` [download] was too. When browsing the Camel, BLOCK variables seemed to be the closest thing to it. I do think your code is more readable. Although it would be nice if it could be simplified, I'm not sure it can be. Unfortunately, your code seems to leave $uids empty. I'll have to dig a bit deeper to see what's going on.	[reply] [d/l]
Re: Re: Re: About BLOCK variables by BrowserUk (Patriarch) on May 24, 2003 at 01:05 UTC
Well, given the the array ref return by $entry->get(...), there's not much that can be done to simplify it further. If you wanted to go for gold on Aristotles patented scale of Yukiness, you could reduce it somewhat more `for my $e ( $searchresult->entries ) { my ($r, $uid) = ($e->get('uid'), $r ? $r->[0] : ()); print "* No uid for the following entry:\n" , Dumper \$e and next unless $uid; ($r, $uids{$uid}{$_}) = ($e->get($_), $r ? $r->[0] : '' ) for @var +s; }` [download] I'll probably get drummed out of the PerlMonks Code-Scribes Guild for even joking about that......but then again, I was never invited to become a member, so what the hay:) However, if its in your power to arrange for $entry->get(..) to return a list instead of an array reference, then the code gets much cleaner.... `for my $entry ( $searchresult->entries ) { my ( $uid ) = $entry->get( 'uid' ); print "* No uid for the following entry:\n" , Dumper \$entry and last unless $uid; ( $uids{$uid}{$_} ) = $entry->get($_) for @vars; }` [download] which is much nicer I think, but can you change get()? If you can, then it would be even better to have it use wantarray to test for the return context and only return the first value of the array (or undef or '') if called in a scalar context (as this seems to be all you want) and the whole array (or ()) in a list context. Then you can remove the parens around the assignments in the last example and it get a little cleaner still. Note: Obviously none of the examples have been tested in context. I've checked individual bits, but... Examine what is said, not who speaks. "Efficiency is intelligent laziness." -David Dunham "When I'm working on a problem, I never think about beauty. I think only how to solve the problem. But when I have finished, if the solution is not beautiful, I know it is wrong." -Richard Buckminster Fuller	[reply] [d/l] [select]
Re: Re: Re: About BLOCK variables by BrowserUk (Patriarch) on May 24, 2003 at 01:27 UTC
Well, given the the array ref return by $entry->get(...), there's not much that can be done to simplify it further. If you wanted to go for gold on Aristotles patented scale of Yukiness, you could reduce it somewhat more `for my $e ( $searchresult->entries ) { my ($r, $uid) = ($e->get('uid'), $r ? $r->[0] : ()); print "* No uid for the following entry:\n" , Dumper \$e and next unless $uid; ($r, $uids{$uid}{$_}) = ($e->get($_), $r ? $r->[0] : '' ) for @var +s; }` [download] I'll probably get drummed out of the PerlMonks Code-Scribes Guild for even joking about that......but then again, I was never invited to become a member, so what the hay:) However, if its in your power to arrange for $entry->get(..) to return a list instead of an array reference, then the code gets much cleaner.... `for my $entry ( $searchresult->entries ) { my ( $uid ) = $entry->get( 'uid' ); print "* No uid for the following entry:\n" , Dumper \$entry and next unless $uid; ( $uids{$uid}{$_} ) = $entry->get($_) for @vars; }` [download] which is much nicer I think, but can you change get()? If you can, then it would be even better to have it use wantarray to test for the return context and only return the first value of the array (or undef or '') if called in a scalar context (as this seems to be all you want) and the whole array (or ()) in a list context. Then you can remove the parens around the assignments in the last example and it get a little cleaner still. Note: Obviously none of the examples have been tested in context. I've checked individual bits, but... Examine what is said, not who speaks. "Efficiency is intelligent laziness." -David Dunham "When I'm working on a problem, I never think about beauty. I think only how to solve the problem. But when I have finished, if the solution is not beautiful, I know it is wrong." -Richard Buckminster Fuller	[reply] [d/l] [select]
Re: Re: Re: Re: About BLOCK variables by Anonymous Monk on May 24, 2003 at 06:22 UTC
After further testing, your original snippet does work and %uids is loaded correctly. Sorry for the false alarm. Unfortunately, the get() method is from one of the CPAN LDAP modules listed in my original snippet. I'm hardly qualified to attempt modifications. Thanks to all for the feedback. I love this place!	[reply]