mirage4d has asked for the wisdom of the Perl Monks concerning the following question:

Greetings all.

There has to be a better way to perform the task below. Essentially I'm just renaming fields returned from an LDAP directory searchfor output - and this works - but there has to be a more efficient way. Using all of these "IF" conditionals seems really clunky. Any suggestions are appreciated. Thanks. 

   @ldapFields = ("unvLocalPhone", "unvLocalAddress", ...);

     foreach $field @ldapFields {

       if ($field eq "unvLocalPhone") {
           $field = "Phone";
       }
       if ($field eq "unvLocalAddress") {
           $field = "Address";
       }

       # Many more ifs follow...

[download]
And so on for about ten fields, which makes for a lot of conditionals. Hoping to find a better way.

Replies are listed 'Best First'.
Re: A more efficient way to do this?
by kennethk (Abbot) on Feb 11, 2010 at 16:15 UTC
    This can be accomplished simply and efficiently using a hash: 

    #!/usr/bin/perl
use strict;
use warning

my @ldapFields = ("unvLocalPhone", "unvLocalAddress", );

my %field_hash = (unvLocalPhone => "Phone",
                  unvLocalAddress => "Address",
                  );
foreach my $field (@ldapFields) {
    if (exists $field_hash{$field}) {
        $field = $field_hash{$field};
    }
}

    [download]

    A hash is an associative array of scalars - it associates a scalar key with exactly one scalar value - and is (IMHO) Perl's single greatest feature. If you are unfamiliar with them, a read through perldata would be a good start.

[reply]
[d/l]
Re: A more efficient way to do this?
by ikegami (Patriarch) on Feb 11, 2010 at 16:13 UTC 
    my %field_name_lkup = (
    unvLocalPhone => 'Phone',
    ...
);

$field = $field_name_lkup{$field};

    [download]
[reply]
[d/l]
Re: A more efficient way to do this?
by toolic (Bishop) on Feb 11, 2010 at 16:16 UTC
    If you want to delete the starting "unvLocal" string from each array element, you can use the substitution operator: 
    s/^unvLocal// for @ldapFields;

    [download]
[reply]
[d/l]
Re: A more efficient way to do this?
by Ratazong (Monsignor) on Feb 11, 2010 at 16:22 UTC

    You could also do:

    foreach $field @ldapFields {
        $field =~ s/unvLocalPhone/Phone/;
        $field =~ s/unvLocalAddress/Address/;
        ...

    [download]

    the solution using hashes has already been presented by others ... see above

[reply]
[d/l]

      Not in Perl, but in Perl you could:

      foreach my $field (@ldapFields) {
        $field =~ s/unvLocalPhone/Phone/;
        $field =~ s/unvLocalAddress/Address/;
        #...
}

      [download]

      although that is probably less efficient in terms of execution time and only marginally better in terms of maintainability and programmer time. Besides, it doesn't do what the OP's code does - consider what happens if an element of @ldapFields were 'unvLocalPhoneNumber' for example.

      True laziness is hard work
[reply]
[d/l]
        All, Thanks for the responses. Yeah, the idea of using a hash had crossed my mind. I'm not very proficient with them, but it did occur to me that it might be the way to go in this situation. However, I should have mentioned in the original post that there will cases where fields will be mapped to the same new field name, i.e.: 
        if ($field eq unvLocalPhone) || ($field eq unvCampusPhone)){
     $field = "Phone";
}

        [download]

        There will several or more cases of this actually. I am wondering if these instances make using a hash a bit more cumbersome?

        Performance is an issue with this script; it is already a bit slow due to the size of the directory being searched. I have read on more than one occasion that hashes, while good coding practice in Perl, are somewhat more resource intensive than simple arrays. Though in this case all of these conditionals may negate any theoretical performance gain achieved by using a simple array. Any thoughts here?

        Thanks again.
[reply]
[d/l]
Re: A more efficient way to do this?
by hbm (Hermit) on Feb 12, 2010 at 13:46 UTC

    One simple improvement would be to change your if-if-if... to if-elsif-elsif... so you stop testing after the first truth.

[reply]

Back to Seekers of Perl Wisdom