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

I posted this question before and I made some arrangements to my code. I'm wondering if this is working or maybe I'm understanding the objective wrong.

on this code I'm trying to call the subroutine twice to print out my desire deck of cards. I'm able to call it once but once I call it a second time, I don't get the desire output. any advice will be thankful.

The objective is to shuffle the deck of cards, just once, with just using pop,shift and push, and print out the top 6 cards. I,m able to accomplish that. Now I want to practice on my subroutine calling. First I want call on the function, after the first hand is dealt, I want to call the shuffling function again before dealing another, a different hand.

#########main scritp#######

#!/usr/bin/perl -w use strict; use diagnostics; my @startingdeck = ("A H","2 H","3 H","4 H","5 H","6 H","7H","8 H", "9 H","10 H","J H","Q H","K H", "A D","2 D","3 D","4 D","5 D","6 D","7 D","8 D", "9 D","10 D","J D","Q D","K D", "A C","2 C","3 C","4 C","5 C","6 C","7 C","8 C", "9 C","10 C","J C","Q C","K C", "A S","2 S","3 S","4 S","5 S","6 S","7 S","8 S", "9 S","10 S","J S","Q S","K S"); @cards = @startingdeck; my $x = 0; my @cards = &shuffle(@cards); @cards = &shuffle(@cards); while($x <= 4){ foreach(@cards){ $_ =~ s/C/Clubs/,s/S/Spades/, s/H/Hearts/,s/D/Diamonds/; print"@cards[$x]\n"; $x++; } }

###############Library script########

sub shuffle{ my @cards = @_; foreach $card(@cards){ my @element1 = (pop(@cards), shift(@cards), pop(@cards)); + my @element2 = (shift(@cards), pop(@cards), shift(@cards)); push(@cards, @element1, @element2); } return @cards; }

Replies are listed 'Best First'.
Re: Perl subroutine update
by jethro (Monsignor) on Apr 01, 2011 at 11:06 UTC

    Read the section "Writing subroutines" in perlintro. If you think you understand that, change the first line of your deck subroutine to

    my @deck= @_;

    Then change your subroutine to only operate on @deck. Don't use @random_card or @starting_deck in this subroutine, you don't need them, you only want to change and shuffle @deck. At the end of your subroutine should be

    return @deck

    Also you when you call your subroutine, use for example

    my @starting_deck = &deck(@starting_deck);

    to shuffle @starting_deck. This is how subroutines are used.

    Then read the section "Arrays" in perlintro. If you think you understand that, look at your subroutine again. If you try out this:

    my $element1 = pop(@deck), shift(@deck), shift(@deck); print "element1 is $element1 \n";

    you will see a problem. $element1 holds only one card, but you probably wanted to pop and shift many cards into it. Consider this:

    my @elements1 = (pop(@deck), shift(@deck), shift(@deck)); print "elements1 is @elements1 \n";

    Now all the elements you shifted and popped off are in @elements1, ready to be added to @deck again. Why the additional parenthesis around the pops and shifts are necessary is an advanced topic, it is because ',' has more than one meaning.

    PS: Don't you think something like shuffle would be a more appropriate name for your subroutine than deck ?

      Jethro,

      Thanks for your time. The reason I did not use shuffle was because, Since shuffle is a built-in function, I thought maybe it would conflict.

        There is no shuffle built-in: shuffle. But even if it had been, you could have used &shuffle.
Re: Perl subroutine update
by jwkrahn (Abbot) on Apr 01, 2011 at 11:32 UTC

    Another problen with your code is that you are modifying @random_card while you are iterating over it in a foreach loop which will cause problems.    As it says in perlsyn: "If any part of LIST is an array, "foreach" will get very confused if you add or remove elements within the loop body, for example with "splice". So don't do that."

Re: Perl subroutine update
by cdarke (Prior) on Apr 01, 2011 at 12:22 UTC
    And another thing....
    sub deck(){
    defines a prototype (that empty set of parentheses) which means "don't accept any arguments". You are then overriding the prototype by calling the subroutine with the '&' prefix. The prototype would still not work anyway because you have defined it after you call it, but you don't want a prototype anyway, just:
    sub deck { ... }
Re: Perl subroutine
by marto (Cardinal) on Apr 01, 2011 at 10:31 UTC
Re: Perl subroutine
by Anonymous Monk on Apr 01, 2011 at 10:31 UTC
    Can you explain this piece of code
    $_ =~ s/C/Clubs/,s/S/Spades/, s/H/Hearts/,s/D/Diamonds/;
    what do you think it does?

      yes that is supposed to match for example the letter C of the deck of cards and switch it with Clubs so when you printout the deck you get for example 7 Clubs instead of 7C.

        It doesn't though, because you are doing the substitution four times on each element, thus introducing a nice echo effect.

        knoppix@Microknoppix:~$ perl -Mstrict -wE ' > my @pack = > map { > my $suit = $_; > map qq{$_$suit}, qw{ J Q K A }; > } qw{ H C D S }; > my $n = 0; > while ( $n < 4 ) > { > foreach ( @pack ) > { > s/C/Clubs/, s/S/Spades/, s/H/Hearts/, s/D/Diamonds/; > } > $n ++; > } > say qq{@pack};' JHeartseartseartsearts QHeartseartseartsearts KHeartseartseartsearts A +Heartseartseartsearts JClubslubslubslubs QClubslubslubslubs KClubslub +slubslubs AClubslubslubslubs JDiamondsiamondsiamondsiamonds QDiamonds +iamondsiamondsiamonds KDiamondsiamondsiamondsiamonds ADiamondsiamonds +iamondsiamonds JSpadespadespadespades QSpadespadespadespades KSpadesp +adespadespades ASpadespadespadespades knoppix@Microknoppix:~$

        I hope this is helpful.

        Cheers,

        JohnGG

Re: Perl subroutine update
by jethro (Monsignor) on Apr 05, 2011 at 11:05 UTC

    Please next time post a new node or even a new question thread instead of editing your old node. It makes all the posts in this thread look silly since they refer to code that isn't there. Also I had problems finding your new script as there was no new node in your node list.

    About your code: The subroutine looks much better now, but you forgot to correct the array problem I was talking about in Re: Perl subroutine update beginning with 'Read the section "Arrays"'. Remember, $element1 is a single value, @element1 would be an array of values. Since you pop/shift more than one value of the @cards and want to store them, you need an array.

    Also, when I said, change all references of @random_card or @starting_deck to @cards in this subroutine, I really meant "in this subroutine only". You don't need to change any @random_card or @starting_deck names outside the subroutine, those are normally different variables. They may have the same name, but they are different and usually you want to use different names to make it obvious that they are different. By changing them you also introduced new bugs. But ok, we can fix it, step by step

    Now, when you debug a script and you get error messages, things get really easy in a way. Usually error messages tell you all you need to know if you just make the effort to read them carefully, try to make sense of them and then correct them until the error message vanishes. Since some errors can lead to further error messages later in the script, it is best to always correct the first error message first.

    So execute your script and you will see a warning that you use @starting_deck[$x] instead of $starting_deck[$x]. If you read the section about arrays, you should know that in perl you access a whole array as @array, but a single element of the array as $array[1]. What you did was use an array slice: @array[1,2,3], also legal, but perl is warning you that it makes no sense to use in this way and you probably meant $starting_deck[$x]. Change this and the warning will go away.

    Now the error message "Global symbol "@cards" requires explicit package name at ./t7.pl line 14" is first. Look at line 14. There is a variable @cards you haven't introduced/declared yet, so add 'my' before it.

    Now that error message should be gone. But a new warning comes up if you restart the script that says ""my" variable @cards masks earlier declaration in same scope at ./t7.pl line 17". Check out line 17. Read the error message and try to make sense out of it. It means that you declare @cards a second time. So remove the 'my' before the second '@cards'.

    When you restart the script the next first error message is "Global symbol "@starting_deck" requires explicit package name at ./t7.pl line 21". If you look closely you will see that before you called the variable "@startingdeck" and not "@starting_deck". So correct that.

    Next error message is "Global symbol "$card" requires explicit package name at ./t7.pl line 33". Check out line 33. You have a variable card that you didn't declare. So put 'my' in front of it and the error message vanishes

    Next message is "Global symbol "@card" requires explicit package name at ./t7.pl line 36". Check out line 36. Again a typing error, you used @card instead of @cards. Change that and your script should finally be running again

    If you are at that point, post your code again in a new node and we can talk about how to find the last two bugs in your script

      hello Jethro, thanks for the time to assist me. I kind of experimented a little bit with the code a different way. if you run it without using strict or diagnostics, I get a close to an answer to what I need. Except that then new set of cards, has to be different than the new one and I'm having problems with my matching the one that turns for example H into Hearts.

      ############ Test.pl###########

      #!/usr/bin/perl my @startingdeck = ("A H","2 H","3 H","4 H","5 H","6 H","7H","8 H", "9 H","10 H","J H","Q H","K H", "A D","2 D","3 D","4 D","5 D","6 D","7 D","8 D", "9 D","10 D","J D","Q D","K D", "A C","2 C","3 C","4 C","5 C","6 C","7 C","8 C", "9 C","10 C","J C","Q C","K C", "A S","2 S","3 S","4 S","5 S","6 S","7 S","8 S", "9 S","10 S","J S","Q S","K S"); my @random_card = @startingdeck; my $x = 0; while($x <= 4){ if (my @cards){ $_ =~ s/C/Clubs, s/S/Spades, s/H/Hearts, s/D/Diamonds; } my @cards = deckof(@cards); my @newcards = decko(@cards); print "@cards[$x] , @newcards[$x]\n"; $x++; }

      #############test-lib.pl##############

      #!/usr/bin/perl sub deckof{ my @cards = @_; foreach $card(@random_card){ my $element1 = (pop(@random_card), shift(@random_card), pop(@r +andom_card)); my $element2 = (shift(@random_card), pop(random_card), shift(@ +random_card)); push(@cards, $element1, $element2); } return @cards; } 1;

        Well, sure, if you turn off warnings, naturally you get no warnings, but you are ignoring things that hint at problems with your code. By removing strict, you get fewer errors, but you also get no information when you mistype variable names. Do you think a script works better if it shows no error message but simple prints the wrong result? This is how politicans solve problems, by removing the warning signs ;-)

        You still haven't changed $element1 to @element1 in this script. Did you notice that you call your subroutine as "decko" in line 21?

        Another problem: you should not use foreach on an array when you change the array inside with pop,shift and push. Normally it leads to fewer loop executions as expected. In your case it is even fortunate that it does so because if it had run for every element in the array, your array would have only 1 or 2 cards left because you are constantly dropping cards (i.e. removing them from the array) with your broken shuffle

        Look at the script I posted to your first post about this problem in Re: Perl Subroutine. Notice that I used a different loop. I did it because foreach is not the right choice here

        sorry for the mistakes, I have been working on this for days and getting kind of frustrated, but I won't be better until I get the code right.

        ################test.pl############

        #!/usr/bin/perl -w use strict; require 'test-lib.pl' my @startingdeck = ("A H","2 H","3 H","4 H","5 H","6 H","7H","8 H", "9 H","10 H","J H","Q H","K H", "A D","2 D","3 D","4 D","5 D","6 D","7 D","8 D", "9 D","10 D","J D","Q D","K D", "A C","2 C","3 C","4 C","5 C","6 C","7 C","8 C", "9 C","10 C","J C","Q C","K C", "A S","2 S","3 S","4 S","5 S","6 S","7 S","8 S", "9 S","10 S","J S","Q S","K S"); my @random_cards= @startingdeck; my $x = 0; foreach(@cards){ $_ =~ s/C/Clubs/, s/S/Spades/, s/H/Hearts, s/D/Diamonds/; } my @cards = deckof(@cards); my @newcards = deckof(@cards); print "@cards[$_], @newcards[$_]\n" for 0..4; $x++;

        ############test-lib.pl############

        #!/usr/bin/perl -w use strict; sub deckof{ my @cards = @_; foreach my $card(1...30){ my @elements1 = (pop(@cards), shift(@cards), pop(@cards)); my @elements2 = (shift(@cards), pop(@cards), shift(@cards)); push(@cards, @elements1, @elements2); } return @cards; } 1;