Beefy Boxes and Bandwidth Generously Provided by pair Networks
Syntactic Confectionery Delight
 
PerlMonks  

Sub Routine Problem

by koolgirl (Hermit)
on Oct 23, 2008 at 06:56 UTC ( [id://718965]=perlquestion: print w/replies, xml ) Need Help??

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

My Dearest Monks. A while ago, I had written my first sub routine, had some malfunctions, asked for your help, learned some valuable lessons and have since moved on. Well, I am now trying to expand that same routine, and it isn't working correctly, and lo and behold...I need your expertise yet again. This is the sub routine:

#!usr/bin/perl use strict; my $guess; print "Enter your number...\n"; chomp($guess = <STDIN>); print "card of $guess is ", &card($guess), "\n"; print "\nRockstar Programming Inc. All Rights Reserved\n"; ## subroutines ### sub card { my @card_map = qw(zero one two three four five six seven eight nine); my $guess; my $i = 0; our $num; our $negative = "negative"; local($num) = @_; if ($card_map[$num]) { $card_map[$num]; # return value } else { $num; # return value } # end if if ($num > 0) { $num = - $num; } # end if if ($card_map[$num]) { $negative . $card_map[$num]; # return value } else { $negative . $num; # return value } # end if } # end sub

The code works, however my output isn't what it should be. I am looking to get the cardinal name returned for each number I enter from 1 - 9, or from -9 - 0, and the same number returned for all other entries. However, the words given in my output, are not matching the numbers entered. Theoretically it should work. I think the problem is in how the input is being matched against the @, because the output is off, but it looks to me as though it shouldn't be doing this?

Any suggestions would be greatly appreciated. Also, as always, please forgive my simplicity, as I am very brand new to the programming world, and I realize my code must be elementary and border on a waste of time for most. I have spent quite a bit of time, reviewing every technique I'm using in this code, in both my llama book, the camel book and on-line docs, and I even have an almost exact example of Randall's, which I tweaked (which is basically what this mostly consists of - tweaked examples of Randall's from the llama book), and all of these say this should work. Any clue?

P.S. Thank you for your time Monks.

UPDATE: Fixed a "typo", thanks Lawliet ;)

Replies are listed 'Best First'.
Re: Sub Routine Problem
by moritz (Cardinal) on Oct 23, 2008 at 07:11 UTC
    if ($card_map[$num]) { $card_map[$num]; # return value } else { $num; # return value } # end if

    Here is (one of your) problem(s).

    $num will only be returned if you either write return $num, or if it's the last statement in your sub, which it isn't.

    Also note that $card_map[$num] will return something for negative values of $num, (in that case it counts the index from the back of the array):

    $ perl -wle 'my @card_map = qw(zero one two three); print $card_map[-2 +]' two

    So you most likely need something like this:

    my $return_value = ''; if ($num < 0) { $return_value = $negative; # continue with the positive value from here on $num = -$num; } # and append other results here: $return_value .= $card_map[$num]; return $return_value;

    (I'm also quite sure that you should forget about local for now, and work with my).

Re: Sub Routine Problem
by andreas1234567 (Vicar) on Oct 23, 2008 at 07:26 UTC
    koolgirl,

    This is a good opportunity to learn unit testing with Test::More et.al. :

    # 718965.pm use strict; use warnings; my @card_map = qw(zero one two three four five six seven eight nine); sub card { my $num = shift; my $negative = "negative"; if ($card_map[$num]) { return $card_map[$num]; } else { return $num; } if ($num > 0) { return $num = -$num; } if ($card_map[$num]) { return $negative . $card_map[$num]; } else { return $negative . $num; } } use Test::More; plan tests => 3; is(card(5), 'five', 'Expect card(5) to return "five"'); is(card(6), 'six', 'Expect card(6) to return "six"'); is(card(-6), 'negative six', 'Expect card(-6) to return "negative six" +'); __END__ $ prove 718965.pm 718965....NOK 3 # Failed test 'Expect card(-6) to return "negative six"' # at 718965.pm line 32. # got: 'four' # expected: 'negative six' # Looks like you failed 1 test of 3. 718965....dubious Test returned status 1 (wstat 256, 0x100) DIED. FAILED test 3 Failed 1/3 tests, 66.67% okay Failed Test Stat Wstat Total Fail Failed List of Failed ---------------------------------------------------------------------- +--------- 718965.pm 1 256 3 1 33.33% 3 Failed 1/1 test scripts, 0.00% okay. 1/3 subtests failed, 66.67% okay.
    --
    No matter how great and destructive your problems may seem now, remember, you've probably only seen the tip of them. [1]
Re: Sub Routine Problem
by Limbic~Region (Chancellor) on Oct 23, 2008 at 14:03 UTC
Re: Sub Routine Problem
by hangon (Deacon) on Oct 23, 2008 at 14:02 UTC

    People often try to overthink a problem. Since you're new to programming, you probably don't fully appreciate that simplification gives clarity. Use as few logical steps as necessary, and remove any clutter as you go along. Here's some tips:

    sub card { my @card_map = qw(zero one two three four five six seven eight nine); # these aren't used, so they should be removed my $guess; my $i = 0; # there is no need for these vars to be global: our $num; our $negative = "negative"; # declare them as lexicals instead my $num; my $negative = "negative"; # declaring $num as lexical you don't need to localize: local($num) = @_; # this works here, but do you really understand what its doing? # hint: list context my ($num) = @_; # this is better my $num = $_[0]: # or this: my $num = shift @_; # commonly shortened (@_ is implicit) to: my $num = shift; # for the rest, notice how much simpler the logic # is in moritz reply above # so putting it all together: sub card{ my $num = shift; my $negative = "negative"; my @card_map = qw(zero one two three four five six seven eight nin +e); my $return_value; if ($num < 0){ $num = -$num; $return_value = $negative; } $return_value .= "$card_map[$num]"; return $return_value; } # and simplified further: sub card{ my $num = shift; my @card_map = qw(zero one two three four five six seven eight nin +e); if ($num < 0){ $num = -$num; return "negative $card_map[$num]"; } return $card_map[$num]; }

    Update: corrected typo

Re: Sub Routine Problem
by scorpio17 (Canon) on Oct 23, 2008 at 14:18 UTC

    Good first attempt!

    Here's my version:

    sub card { my $num = shift; my $result; my @card_map = qw(zero one two three four five six seven eight nine) +; if ($num < 0) { $result .= "negative "; $num = -$num; } if ($card_map[$num]) { $result .= $card_map[$num]; } else { $result .= $num; } return $result; }
Re: Sub Routine Problem
by apl (Monsignor) on Oct 23, 2008 at 10:37 UTC
    The code works, however my output isn't what it should be.
    That means the code does not work. You probably meant "The code compiles".

    (Just being pedantic; sorry.)
      We could get pedantic and look at the word "compiles," too.
Re: Sub Routine Problem
by blazar (Canon) on Oct 23, 2008 at 19:15 UTC

    I personally believe that "simplicity is the ultimate sofistication" (Cit.) - indeed I had serious difficulties wandering through your code to understand what it does. Said this, and having seen code from others in this thread too, I think that your sub may be made just as simple as:

    { my @card_map = qw(zero one two three four five six seven eight nin +e); sub card { my ($n, $pre) = map { $_< 0 ? (-$_, 'minus ') : ($_, '') } shi +ft; $pre . ($card_map[$n] // $n); } }

    Here, I preferred to move @card_map out of the sub because I see no point assigning to it each time the sub is called. (I also changed "negative" to "minus" which I think is proper English.)

    Update: Limbic~Region /msg'd me to point out that probably "it will be a while before state declared variables become common place due to compatability issues but that would be his preference here." Indeed, he's right, and I'm using 5.10.0's // (although in this case || would suffice) under use 5.010 anyway, so I thought that for completeness I may rewrite the sub in that form as well:

    This actually led me to discover something that I and appearently Limbic~Region didn't even remotely suspect, but is well documented in perldoc perlsub:

    When combined with variable declaration, simple scalar assignment to state variables (as in state $x = 42 ) is executed only the first time. When such statements are evaluated subsequent times, the assignment is ignored. The behavior of this sort of assignment to non-scalar variables is undefined.

    Well, to be very fussy, the documentation is not entirely consistent with the implementation since the former says that the behaviour of the assignment is "undefined" whereas the latter strictly prohibits it - and I checked, to be sure, that it is not a strict thing: even if you don't use it, compilation fails.

    --
    If you can't understand the incipit, then please check the IPB Campaign.
Re: Sub Routine Problem
by Utilitarian (Vicar) on Oct 23, 2008 at 14:02 UTC
    Try running the following code:
    #! /bin/perl -w use strict; &routine(7); sub routine{ my $array_length=@_; my ($value)=@_; my $specific_value=$_[0]; print "This function was called with $array_length argument(s) and +a value of $value\n the value can also be accessed from the array dir +ectly and returns $specific_value\n"; }
    Arrays called in a scalar context return their size. Hope this helps, Utilitarian
Re: Sub Routine Problem
by koolgirl (Hermit) on Oct 24, 2008 at 06:20 UTC

    Thank you very much, Monks. The examples of code really helped point out some seemingly obvious mistakes, however, after spending two days staring at code, and half of that time de-bugging the part you think you can "get", your vision becomes decidedly unclear :\

    I know I still have quite a bit to learn, and please understand, that being able to de-bug my code efficiently, on my own, is a skill I am strongly striving for, yet apparently very far away from reaching...which is just as annoying as the problem itself, if not more so. I thought the ability to code would be the big hump, however, it seems as though the ability to be able to spot one's own mistakes is the real battle. However, every time ya'll help point things out to me, I do get closer to that point, it just seems as though the road is incredibly long at this level of understanding.

    I thank all of you for your time, attention, code examples and the push each reply gave me towards achieving the ability to see my own mistakes. Of course further in my venture I will want to for see them before they are made, but wow, let's not get too far ahead of ourselves on that one! :)

      koolgirl,
      One of the most effective things you can do to quickly identify and resolve issues with your code is by adopting a consistent methodology to writing the code in the first place. For that, I would recommend reading what I wrote at Re: Refactoring a large script. If you can avoid introducing the bug then you don't have to spend time fixing it.

      When you do need to start bug hunting, many people like brian's Guide to Solving Any Perl Problem. I personally have my own approach which I haven't bothered to document. Often times the biggest help is being able to visualize what's going on - How can I visualize my complex data structure?.

      Again, I have my own approach but one common technique I employ is generously sprinkled print statements. If you have addressed all the issues raised by strictures and warnings but your code is still not producing the correct output, find out where your assumptions failed using the bi-section method. Pick some point in the middle of the execution of your code to output the values of your data up to that point. It doesn't matter if all the values are right or wrong - you have eliminated half of your code as being the source of the problem. Either pick the middle execution point before or after (depending on the correctness of the data values) and wash/rinse/repeate the process.

      Cheers - L~R

Log In?
Username:
Password:

What's my password?
Create A New User
Domain Nodelet?
Node Status?
node history
Node Type: perlquestion [id://718965]
Approved by lamp
help
Chatterbox?
and the web crawler heard nothing...

How do I use this?Last hourOther CB clients
Other Users?
Others browsing the Monastery: (7)
As of 2024-04-23 11:03 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found