| [reply]
[d/l]
[select] |
Greetings, hozefa,
Your program seems to run fine, and (I'm assuming) does exactly what you want it to. So the following are all merely suggestions; as you have already achieved the primary goal of a working program ...
- On Windows (at least on the systems I've used, ActiveState Perl with Win2000 or XP), it doesn't seem to matter where whether you specify a shebang line; Perl will still execute successfully. However, your shebang (#!c:perl/perl.exe) may be misleading, as it's unlikely that the executable exists in the Perl top directory (it's more likely in C:\Perl\Bin).
- I find it easier to read a program with proper indentation. It's up to you whether you use:
if ($sign==5) {
print "the Multiplication Table For $num1 and $num2 is \n";
for (1 ..$num2) {
print "$_ X $num1 = ", $_ * $num1, "\n";
}
}
(which I like) or:
if ($sign==5)
{
print "the Multiplication Table For $num1 and $num2 is \n";
for (1 ..$num2) {
print "$_ X $num1 = ", $_ * $num1, "\n";
}
}
but I think they are both preferrable to:
if ($sign==5)
{
print "the Multiplication Table For $num1 and $num2 is \n";
for (1 ..$num2)
{
print "$_ X $num1 = ", $_*$num1, "\n";
}
}
which is misleading and erratic.
- When doing == comparisons, a nice trick is to put the constant on the left. (This is completely an optional suggestion; I would never fault a program for not doing it). I find it helpful because if you ever mistype "==" as "=", then the compiler will catch the error, as you can't assign something to a constant (if (1=$sign) { ... }).
- Check your program for spelling (eg. "Mulitiplication"), and consider better variable names (eg. "$sign" might be better labelled "$operation").
- You might want to use if ... elsif ... instead of individual "if" statements. They don't change the functionality, but do allow some conditionals not to be evaluated (not necessarily a slow down in a small program, but a good practice nonetheless), and with a tighter grouping, may make the program easier to read:
if (1 == $sign) {
print "The Sum of the Numbers $num1 + $num2 is ", $num1 + $num2, "
+\n";
} elif (2 == $sign) {
print "The Difference of the numbers $num1 - $num2 is ",$num1 - $n
+um2, "\n";
} elsif (3 == $sign) {
print "The Product of $num1 X $num2 is ",$num1 * $num2 , "\n";
} elsif (4 == $sign) {
print "The Division of $num1 / $num2 is ", $num1 / $num2 , "\n";
} elsif (5 == $sign) {
print "the Multiplication Table For $num1 and $num2 is \n";
for (1 ..$num2) {
print "$_ X $num1 = ", $_ * $num1, "\n";
}
}
Good luck in all your programming tasks. May you enjoy programming in Perl as much as many of us have!
s''(q.S:$/9=(T1';s;(..)(..);$..=substr+crypt($1,$2),2,3;eg;print$..$/
| [reply]
[d/l]
[select] |
both replies above have good advice .. reading through your code I envisioned slightly different code reduction/dispatch tables and thought I'd share in the spirit of TMTOWTDI (a term which, if you haven't seen by name i'm sure you've already seen in use)
One thought, was no need to use numbers in your menu -- just have them enter the operation. And in general, I like to have one printf statement if they're all similar in form -- this separates content from logic and makes it much easier to modify in the future.
# input num1
# input num2
# First, setup a hash that defines what's can be done and it's attribu
+tes.
my %ops = (
'+' => { label => 'Sum' },
'-' => { label => 'Difference' },
'*' => { label => 'Product' },
'/' => { label => 'Division' },
'M' => { label => 'Multiplication', table => 1, op => '*' },
);
while (1){ #adding a loop around this
# using a here-doc to do the output
print <<EOS;
Please enter the Maths that you want to perform:
+ - * /
M = Mulitiplication Table
EOS
chomp($sign=<STDIN>);
last if exists $ops{$sign}; # see if we got something that we confi
+gured in the hash
}
my $op = $ops{$sign};
if( $op->{table} ){
printf "the %s Table For %d and %d is \n", $op->{label}, $num1, $num
+2;
printf("%d %s %d = %f\n", $_, $op->{label}, $num1, eval($_.$op->{op}
+.$num1) ) for 1 ..$num2;
}else{
printf "The %s of the Numbers %d %s %d is %f\n",
$op->{label},
$num1, $sign, $num2,
eval "$num1 $sign $num2",
;
}
Notice, for example, how trivial it is to change the output (e.g. "____ of the numbers" to "___ for") or to extend it (e.g. add an option to display the Addition table).
one caveat about eval -- it's use here is ok becuase the input had been checked, so we know it's _only_ got a few (safe) values. A more robust dispatch table could provide the actual subroutine to call (Super Search for 'dispatch table' and 'function references' for more) | [reply]
[d/l] |
I dislike using eval myself unless absolutely necessary. Why not just use linked subs:
use strict;
use warnings;
my (%ops, $first, $second, $op, $f);
%ops = (
'+' => { 'label' => 'Sum', 'op' => sub { return $_[0] + $_[1]; } }
+,
'-' => { 'label' => 'Difference', 'op' => sub { return $_[0] - $_[
+1]; } },
'*' => { 'label' => 'Product', 'op' => sub { return $_[0] * $_[1];
+ } },
'/' => { 'label' => 'Quotient', 'op' => sub { return $_[0] / $_[1]
+; } }
);
$ops{'M'} = { 'label' => 'Multiplication', 'table' => 1, 'op' => $ops{
+'*'}{'op'} };
sub getval {
chomp($_ = <STDIN>);
($_) = m/($_[0])/ if $_[0];
return $_;
}
while (1) {
print "Enter the first number: ";
last if !($first = getval('\d+'));
print "Enter the second number: ";
last if !($second = getval('\d+'));
print <<TEXT;
Please enter the operation that you want to perform:
+ - * /
M = Mulitiplication Table
TEXT
last if !$ops{$op = getval()};
print "\n";
if (!$ops{$op}{'table'}) {
print "$ops{$op}{'label'} of $first $op $second is ",
$ops{$op}{'op'}->($first, $second);
}
else {
print ' ', join ' ', 1..$second;
for $f (1..$first) {
print "\n";
print join ' ', $f, map { $ops{$op}{'op'}->($f, $_) } 1..$
+second;
}
}
print "\n\n";
}
| [reply]
[d/l] |