$var = "something";
    $var = "otherthing" if EXPR;
[download]

as

    $var = EXPR ? "otherthing" : "something";
[download]

But some people don't like ?:. I sometimes use the former form myself as well.

Abigail

First off: Assigning '' to $uid here is both mildly confusing of the issue, and redundant. The reason is that my $uid; will implicitly set $uid to undef, which is false.

However, that's not the problem, as far as I can see. There's two possiblities. The first is that $entry->get($_) might not return the same thing all three times you call it. Presumably, you'd know if this were a possiblity, so I won't talk much about it. The second possiblity is that it returns somthing that isn't an array reference, but is true.

I'd do this somthing like this:

for my $entry ($searchresult->entries) {
   my $uid = $entry->get('uid');
   if (!$uid || !UNIVERSAL::isa($uid, 'ARRAY')) {
     print "*** No uid for the following  entry:\n";
     print Dumper(\$entry);
   }
   $uid = ${$uid}[0];
   if (!$uid) {
     print "*** No uid for the following  entry:\n";
     print Dumper(\$entry);
   }
   for (@vars) {
         $uids{$uid}{$_} = '';
         $uids{$uid}{$_} = ${$uid}[0];
   }
}
[download]

Note that UNIVERSAL::isa($foo, 'ARRAY'); will tell you if $foo is a reference to an array (or if it is a reference to somthing else blessed into a class named ARRAY, which I find terribly unlikely).

Actually I don't think 'isa' is an issue. The structures are generated by the CPAN LDAP modules (as are the methods for accessing it) and the snippet is pretty close to the Pod examples. But since I didn't include the actual setup calls to the LDAP query, it probably made it difficult to tell what's going on. Sorry, it didn't seem relevant to me at the time the post was formulated.

I assume you made a cut/paste error for the second for loop. The first line is redundant since the second assignment doesn't include the required conditional test for definedness. It also omits the get method so you're actually assigning the same value(s) to each of the different @var values of each $uid instance. E.g.:

$uids{$uid}{mail} = $uids{$uid}{cn} = ${$uid}[0]
[download]

which shouldn't be the case.

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.

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.

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

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