if (defined $ARGV[0] && defined $ARGV[1] && defined $ARGV[2] && define
+d $ARGV[3]) {
...
}
else { #Die if less than 4 args given
die "Please define at least 4 arguments\n";
}
I presume from what you said above that you're always hitting the else from this code. Without knowing how you're calling your script I can't guess why this is going wrong. However I'll offer you some ideas on how you can generally improve your code.
1.
In the if statement I quoted above, all you're trying to see is whether there are 4 arguments. This can be done in a much neater way:
if(@ARGV < 4) {
die "Please define at least 4 arguments\n";
}
# I must have 4 arguments at this point.
Note that I've flipped your if/else statement here too. This helps you reduce the number of levels of indentation and lets you handle the error case immediately rather than in a completely unrelated piece of code.
Of course what you really want is to check that @ARGV is less than 3 because in the case of a square you only ever ask for one more value.
2.
Use hashes! If you ever see yourself writing code like this:
if ($ARGV[0]=="tri") {
my $figure="triangle";
}
elsif ($ARGV[0]=="cir") {
my $figure="circle";
}
...
if ($ARGV[1]=="area") {
my $op="area";
if ($figure=="rectangle") { my $reqargs=2; }
elsif ($figure=="square") { my $reqargs=1;}
elsif ($figure=="triangle") { my $reqargs=2; }
elsif ($figure=="circle") { my $reqargs=1; }
}
you should probably think: "hmm, this can be represented in a hash." For example:
my $shape = shift @ARGV; # take first argument off ARGV
my %shapes = (
"tri" => "triangle",
"cir" => "circle",
"req" => "rectangle",
"sqr" => "square"
);
my $figure = $shapes{$shape} or die "Unknown shape $shape\n";
The advantage of this is that you can store other, more interesting information in the same hash which will save you time later.
my $shape = shift @ARGV; # take first argument off ARGV
my $op = shift @ARGV; # take second argument off ARGV
my %shapes = (
"tri" => {
name => "triangle",
area => 2, # required number of args
peri => 3,
hypo => 2,
},
"sqr" => {
name => "square",
area => 1,
peri => 1,
},
"rec" => {
name => "rectangle",
area => 2,
peri => 2,
},
"cir" => {
name => "circle",
area => 1,
peri => 1,
},
);
# Get full name of figure.
my $figure = $shapes{$shape}{name} or die "Unknown shape: $shape\n";
# Determine how many arguments we require.
my $reqargs = $shapes{$shape}{$op} or die "Unknown operator $op".
" or $op does not work with shape $figure";
3.
Rethink your need for $refsum. This is one point where if/elsif/else would make your code a whole bunch more readable. And yes, you could even move the code for the if/elsif/else into your hash if you wanted to, but I thought it would be better to start out easy.
4.
As others have said, use strict. If you were using strict you would have been told that $answer was unknown where you check whether it's undefined. As others have said this is because you're doing this:
elsif ($refsum==12.2) {
my $diameter=$ARGV[3];
my $answer=$diameter*$pi;
} # $answer stops existing HERE
You need to declare $answer above that big mess and then do:
elsif ($refsum==12.2) {
my $diameter=$ARGV[3];
$answer=$diameter*$pi;
}
5.
Double check your formulas. Since you're using $refsum it's hard to see what you're actually calculating but some of them are wrong. The area of a triangle is half what your code will report, for example.
Anyway, I've put all of these ideas into practice and this is the script I get. Compare it closely to what you've done and feel free to ask about any changes.
#!usr/bin/perl -w
use warnings;
use strict;
print "Welcome to Will's Plane Geometry Calculator.\nThe ".
"first argument should be tri, cir, rec, or sqr.\n".
"The second argument should be area, peri, or ".
"hypo.\nIf you give hypo as the second argument, ".
"the next two arguments should ".
"be the known sides of the traingle.\nthe peri arg ".
"will also find circumference ".
"if cir is given for the figure. Specify the ".
"diameter to find this.\nPi is approximated at ".
"3.141592654 (simply change my pi if a different ".
"approximation is needed)\nTo find area of a circle ".
"specify the radius.\nI think you can figure ".
"out the rest.\n", "-" x76, "\n";
my $pi=3.141592654;
if (@ARGV < 3) {
die "Please define at least 3 arguments\n";
}
my $shape = shift @ARGV; # take first argument off ARGV
my $op = shift @ARGV; # take second argument off ARGV
my %shapes = (
"tri" => {
name => "triangle",
area => 2,
peri => 3,
hypo => 2,
},
"sqr" => {
name => "square",
area => 1,
peri => 1,
},
"rec" => {
name => "rectangle",
area => 2,
peri => 2,
},
"cir" => {
name => "circle",
area => 1,
peri => 1,
},
);
my %operators = (
"area" => "area",
"peri" => "perimeter",
"hypo" => "hypotenuse",
);
# Get full name of figure.
my $figure = $shapes{$shape}{name} or die "Unknown shape: $shape\n";
# Determine how many arguments we require.
my $reqargs = $shapes{$shape}{$op} or die "Unknown operator $op".
" or $op does not work with shape $figure";
# Get our operator name
my $operator = $operators{$op};
print "\n INFO PRINTOUT FROM SCALARS:\n FIGURE: $figure\n ",
" OPERATION: $operation\n REQUIRED ARGS: ".
" $reqargs\n\n";
# Since we know $reqargs, check that we can go ahead at
# this point.
if(@ARGV < $reqargs) {
die "Not enough arguments provided for this ".
"function.";
}
my $answer;
if($shape eq "tri") {
if($op eq "area") {
my ($base, $height) = @ARGV;
$answer = 0.5*$base*$height;
}
elsif($op eq "peri") {
my ($side1, $side2, $side3) = @ARGV;
$answer = $side1 + $side2 + $side3;
}
elsif($op eq "hypo") {
my ($side1, $side2) = @ARGV;
$answer = sqrt( $side1*$side1 +
$side2*$side2 );
}
}
elsif($shape eq "cir") {
if($op eq "peri") {
my ($diameter) = @ARGV;
$answer = $diameter*$pi;
}
elsif($op eq "area") {
my ($radius) = @ARGV;
$answer = $radius*$radius * $pi;
}
}
elsif($shape eq "sqr") {
if($op eq "area") {
my ($side) = @ARGV;
$answer = $side*$side;
}
elsif($op eq "peri") {
my ($side) = @ARGV;
$answer = $side*4;
}
}
elsif($shape eq "rec") {
if($op eq "area") {
my ($length, $width) = @ARGV;
$answer = $length*$width;
}
elsif($op eq "peri") {
my ($length, $width) = @ARGV;
$answer = $length*2 + $width*2;
}
}
if (defined $answer) {
print "\n\n","-" x76,"\n","Calculations Completed ".
"Successfully!\n";
print "Your answer is: $answer\n";
}
else {
print "\n\n","-" x76,"\n","Calculations Completed.".
"\n Operation Unsuccessful.\n Cause ".
"Unknown.\n";
}
print "pi is $pi\n";
I hope this helps.
jarich
-
Are you posting in the right place? Check out Where do I post X? to know for sure.
-
Posts may use any of the Perl Monks Approved HTML tags. Currently these include the following:
<code> <a> <b> <big>
<blockquote> <br /> <dd>
<dl> <dt> <em> <font>
<h1> <h2> <h3> <h4>
<h5> <h6> <hr /> <i>
<li> <nbsp> <ol> <p>
<small> <strike> <strong>
<sub> <sup> <table>
<td> <th> <tr> <tt>
<u> <ul>
-
Snippets of code should be wrapped in
<code> tags not
<pre> tags. In fact, <pre>
tags should generally be avoided. If they must
be used, extreme care should be
taken to ensure that their contents do not
have long lines (<70 chars), in order to prevent
horizontal scrolling (and possible janitor
intervention).
-
Want more info? How to link
or How to display code and escape characters
are good places to start.