Beefy Boxes and Bandwidth Generously Provided by pair Networks
Problems? Is your data what you think it is?
 
PerlMonks  

Perlish way of doing this

by mkirank (Chaplain)
on Dec 08, 2004 at 11:04 UTC ( [id://413151]=perlquestion: print w/replies, xml ) Need Help??

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

Replies are listed 'Best First'.
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:

    1. you want to use as lookup table and have to iterate the whole thing each time with grep.
    2. Dont care about the order, only the presence.
    3. 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
      Small suggestion to reduce redundancy:
      for my $h (\%a1, \%a2) { print join ',', map{ exists $h->{ $_ } ? $_ : '##' } @a; }
      Knitpicking is the only way to perfection.
      --kap

        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
Re: Perlish way of doing this
by fglock (Vicar) on Dec 08, 2004 at 11:54 UTC

    A bit more perlish. Note that %a is not really necessary. And the keys could be pre-sorted.

    #!/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

Re: Perlish way of doing this
by htoug (Deacon) on Dec 08, 2004 at 11:44 UTC
    It would probably help if you explained *what* the program is trying to do. Of course we can all use our copious free time to analyse it and discover that your program just blorfurgates the quadruggles, but it would be a lot easier if you had explained it.

    There is more than one way to do it - especially in perl!

      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.

Re: Perlish way of doing this
by Random_Walk (Prior) on Dec 08, 2004 at 12:08 UTC

    how about

    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.

      Here is an improvement on the above. I do not use map to fill the two tmp arrays as they would need to be itterated over again to find the case where both are blank. May as well only loop once (I added GG to test this case too).

      #!/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.

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.

    --kap
Re: Perlish way of doing this
by mkirank (Chaplain) on Dec 08, 2004 at 12:48 UTC
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.

      If you look at the program written ,It has a lot of if ,elsif blocks ...can that be avoided ?

Log In?
Username:
Password:

What's my password?
Create A New User
Domain Nodelet?
Node Status?
node history
Node Type: perlquestion [id://413151]
Approved by Happy-the-monk
Front-paged by htoug
help
Chatterbox?
and the web crawler heard nothing...

How do I use this?Last hourOther CB clients
Other Users?
Others having an uproarious good time at the Monastery: (4)
As of 2024-04-24 05:48 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found