> -----Original Message-----
> From: Michael Carmody [mailto:[EMAIL PROTECTED]]
> Sent: Thursday, July 26, 2001 2:30 AM
> To: [EMAIL PROTECTED]
> Subject: Tear me to shreds please...
> ...
Regarding this section of the code:
> $c = ($data[4][$i] =~ s/\//\//g);
> if ($c eq "")
> {
> print LDIF "sn: $data[4][$i]\n";
> }
> else
> {
> @array = split(/\//,$data[4][$i]);
> $j = scalar(@array);
> $j--;
> for $d (0 .. $j)
> {print LDIF ("sn:",$array[$d],"\n");}
> }
It appears that what you are trying to do is print the parts of $data[4][$i]
as delimited by forward slashes on individual lines, with each line prefixed
by "sn: ". Is that correct? If so, this entire block of code can be replaced
with one line:
print LDIF "sn: $_\n" for split /\//, $data[4][$i];
Try to get away from thinking in C terms. Perl absolutely excels at dealing
with managing lists of things and iterating over those lists in various
ways.
Some comments:
1. You are first trying to decide whether the string contains any slashes.
If not, you print the whole string. Otherwise, you split the string into
multiple parts. This is not necessary. The split function will handle a
string with no delimiters by simply returning a list consisting of one
element: the entire string. Also, you are using the operation
s/\//\//g
to test whether the string has any slashes. This is incredibly inefficient,
as you are asking perl to replace a slash with a slash and the /g modifiers
forces perl to do so for every instance in the string. But you don't care
how many slashes are in the string, just whether there is one. The simpler
test would be
/\// (or m!/! which is possibly more readable)
But again, the whole test is unneeded.
2. You use
if ($c eq "")
to test the return from the s/// operation. This makes me uncomfortable. You
really want to test whether $c does not have a value. I would use
unless ($c)
3. You use the following code to iterate over @array:
@array = split(/\//,$data[4][$i]);
$j = scalar(@array);
$j--;
for $d (0 .. $j) {
print LDIF ("sn:",$array[$d],"\n");
}
First, $j is unnecessary. If you want the last index in the array, use
$#array:
for $d (0 .. $#array)
Second, don't use subscripting if you just want to iterate over the array.
Use perl's iteration capabilties:
for (@array) {
print LDIF ("sn: $_\n");
}
Third, if the array body is a simple one-liner, use the for statement
modifier instead:
print LDIF "sn: $_\n" for @array;
Finally, the intermediate array is unnecessary. Use the output of split
directly:
print LDIF "sn: $_\n" for split /\//, $data[4][$i];
This eliminates several lines of code, is more readable, and uses no
additional variables.
--
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
