mkirank has asked for the wisdom of the Perl Monks concerning the following question:
#!/usr/bin/perl
use strict;
my %a=(
AA=>'Y',
BB=>'Y',
CC=>'Y',
DD=>'Y',
EE=>'Y',
FF=>'Y'
);
my @a1=('AA','DD','EE');
my @a2=('AA','BB','CC','FF');
my(@tmp1,@tmp2);
foreach my $xx (sort (keys(%a))) {
$a = grep (/$xx/, @a1);
$b = grep (/$xx/, @a2) ;
if (($a && $b) || (!$a && !$b)) {
push (@tmp1,$xx);
push (@tmp2,$xx);
} elsif ($a && (!$b)) {
push (@tmp1,$xx);
push (@tmp2,'##');
} elsif (!$a && ($b)) {
push (@tmp1,'##');
push (@tmp2,$xx);
}
}
print join(',',@tmp1);
print "\n";
print join(',',@tmp2);
print "\n";
Is there a more perlish way of doing this ?
What i want to do here is create 2 arrays tmp1 and tmp2 whose length is that of the size of (keys of %a) and
if an element exists in both @a1 and @a2 then they will be put to tmp1 and tmp2 ,
If one of the array (a1 or a2)dose'nt contain the key then a ## should be Inserted there,
This is working fine and I just wanted to know if there is anything that can be improved in the code
Re: Perlish way of doing this
by BrowserUk (Patriarch) on Dec 08, 2004 at 11:50 UTC
|
You're storing your data in the wrong structures.
You never use the values in the hash %a; you never use it's lookup ability; and you want to iterate it in order.
Your arrays @a1 and @a2:
- you want to use as lookup table and have to iterate the whole thing each time with grep.
- Dont care about the order, only the presence.
- Iterate, only for the greps, never for the ordering.
By reversing the type of structures used you get:
#! perl -slw
use strict;
my %a=( AA=>'Y', BB=>'Y', CC=>'Y', DD=>'Y', EE=>'Y', FF=>'Y' );
my @a1=('AA','DD','EE');
my @a2=('AA','BB','CC','FF');
my @a = sort keys %a;
my %a1; @a1{ @a1 } = ();
my %a2; @a2{ @a2 } = ();
print join ',', map{ exists $a1{ $_ } ? $_ : '##' } @a;
print join ',', map{ exists $a2{ $_ } ? $_ : '##' } @a;
__END__
[11:44:17.92] P:\test>413151
AA,##,##,DD,EE,##
AA,BB,CC,##,##,FF
Examine what is said, not who speaks.
"But you should never overestimate the ingenuity of the sceptics to come up with a counter-argument." -Myles Allen
"Think for yourself!" - Abigail
"Time is a poor substitute for thought"--theorbtwo
"Efficiency is intelligent laziness." -David Dunham
"Memory, processor, disk in that order on the hardware side. Algorithm, algorithm, algorithm on the code side." - tachyon
| [reply] [d/l] |
|
Small suggestion to reduce redundancy:
for my $h (\%a1, \%a2) {
print join ',', map{ exists $h->{ $_ } ? $_ : '##' } @a;
}
Knitpicking is the only way to perfection.
| [reply] [d/l] |
|
You're right!
... except now that you have to explain not only why the OP should be using an array instead of a hash, and hashes instead of arrays, but also why you're escaping your percent signs and why your $h is firing an arrow at the head of a cartoon character with a one eye closed and a dollar sign in the other :)
Examine what is said, not who speaks.
"But you should never overestimate the ingenuity of the sceptics to come up with a counter-argument." -Myles Allen
"Think for yourself!" - Abigail
"Time is a poor substitute for thought"--theorbtwo
"Efficiency is intelligent laziness." -David Dunham
"Memory, processor, disk in that order on the hardware side. Algorithm, algorithm, algorithm on the code side." - tachyon
| [reply] |
|
Re: Perlish way of doing this
by fglock (Vicar) on Dec 08, 2004 at 11:54 UTC
|
#!/usr/bin/perl
use strict;
my @a = qw( AA BB CC DD EE FF );
my @a1 = qw( AA DD EE );
my @a2 = qw( AA BB CC FF );
my %a = map { $_ => 1 } @a;
my %a1 = map { $_ => 1 } @a1;
my %a2 = map { $_ => 1 } @a2;
my (@tmp1, @tmp2);
@tmp1 = map { exists $a1{$_} ? $_ : '##' } sort keys %a;
@tmp2 = map { exists $a2{$_} ? $_ : '##' } sort keys %a;
print join(',',@tmp1);
print "\n";
print join(',',@tmp2);
print "\n";
update: removed unnecessary parentheses
| [reply] [d/l] |
Re: Perlish way of doing this
by htoug (Deacon) on Dec 08, 2004 at 11:44 UTC
|
| [reply] |
|
Ahhh, now I see. Ie after you updated the question.
I think that it could be simplified by noting that the two tmp-arrays can be created given only one of the input arrays - @tmp1 depends only on @a1 and @tmp2 only on @a2.
Your loop could be shortened to something like:
for my $elem (sort keys %a) {
push @tmp1, (grep($elem eq $_} @a1) ? $elem : "##";
push @tmp2, (grep($elem eq $_} @a2) ? $elem : "##";
}
That satisfies your explanation.
But your code seems to indicate that the value is put into the tmp-arrays also if the value is present in neither the @a1 or @a2. Unfortunately your code does not have an example of this, so it cannot be tested. | [reply] [d/l] |
Re: Perlish way of doing this
by Random_Walk (Prior) on Dec 08, 2004 at 12:08 UTC
|
print "AA,##,##,DD,EE,##\nAA,BB,CC,##,##,FF\n" # ;-)
What I am saying is that it is obvious what you are doing but why ? Which parts of this are external inputs and what can we play with ? Why do you assign the value 'Y' to all the values of a hash and later sort the keys ? An array would look to be a more obvious solution or even just do this
#!/usr/local/bin/perl -w
use strict;
my @a1=('AA','DD','EE');
my @a2=('AA','BB','CC','FF');
my(@tmp1,@tmp2);
foreach my $xx ('A'..'F') {
$xx=$xx x 2;
$a = grep (/$xx/, @a1);
$b = grep (/$xx/, @a2) ;
if (($a && $b) || (!$a && !$b)) {
push (@tmp1,$xx);
push (@tmp2,$xx);
} elsif ($a && !$b) {
push (@tmp1,$xx);
push (@tmp2,'##');
} else {
push (@tmp1,'##');
push (@tmp2,$xx);
}
}
print join(',',@tmp1);
print "\n";
print join(',',@tmp2);
print "\n";
Why the ddoouubbllee letters ? Can we do it all in single letters and double them up at print time ? Will one of the values always be present in either @a1 or @a2 ? Why not put these into hashes to save that greping ?
#!/usr/local/bin/perl -w
use strict;
my (%a1, %a2, @tmp1,@tmp2);
$a1{$_}=1 for ('AA','DD','EE');
$a2{$_}=1 for ('AA','BB','CC','FF');
foreach my $xx ('A'..'F') {
$xx=$xx x 2;
if (($a1{$xx} && $a2{$xx}) || (!$a1{$xx} && !$a2{$xx})) {
push (@tmp1,$xx);
push (@tmp2,$xx);
} elsif ($a1{$xx}) {
push (@tmp1,$xx);
push (@tmp2,'##');
} else {
push (@tmp1,'##');
push (@tmp2,$xx);
}
}
print join(',',@tmp1);
print "\n";
print join(',',@tmp2);
print "\n";
This is not as simple as some of the examples above but then this one handles the case where neither a1 or a2 contains the value and correctly puts XX into both output streams.
Cheers, R. | [reply] [d/l] [select] |
|
#!/usr/bin/perl -w
use strict;
my (%a1, %a2, @tmp1,@tmp2);
$a1{$_}=1 for ('AA','DD','EE');
$a2{$_}=1 for ('AA','BB','CC','FF');
foreach my $xx ('A'..'G') {
$xx=$xx x 2;
push @tmp1,($a1{$xx} ? $xx : 'XX');
push @tmp2,($a2{$xx} ? $xx : 'XX');
$tmp1[-1]=$tmp2[-1]=$xx if $tmp1[-1] eq $tmp2[-1]
}
print join(',',@tmp1);
print "\n";
print join(',',@tmp2);
print "\n";
Cheers, R. | [reply] [d/l] |
Re: Perlish way of doing this
by kappa (Chaplain) on Dec 08, 2004 at 11:40 UTC
|
print "AA,##,##,DD,EE,##\nAA,BB,CC,##,##,FF\n";
Could not resist. This is the most perlish way of accomplishing a task which should be derived from the code.
| [reply] [d/l] |
Re: Perlish way of doing this
by mkirank (Chaplain) on Dec 08, 2004 at 12:48 UTC
|
| [reply] |
Re: Perlish way of doing this
by dave_the_m (Monsignor) on Dec 08, 2004 at 11:15 UTC
|
Is there a more perlish way of doing this
Of doing what?
Dave.
| [reply] |
|
If you look at the program written ,It has a lot of if ,elsif blocks ...can that be avoided ?
| [reply] |
|
|