Here's your original code:
#!/usr/bin/perl
use strict;
use warnings;
my $name;
my $upto;
my @response = ("That's interesting.","Eh, boring...","Ugh.","Cool! K
+eep up the good work!");
sub april {
print "Hi, my name is April! What's yours? ";
chomp($name = <STDIN>);
print "Hi ", $name, ", nice to meet you! What are you up to? ";
for (;;) {
chomp($upto = <STDIN>);
$upto eq "nothing" and last;
print $response[rand @response], "\n";
print "Anything else? ";
}
print "Goodbye, $name!\n";
}
april();
And here's how I rewrote it:
#!/usr/bin/perl
use strict;
use warnings;
my @response = (
"That's interesting.",
"Eh, boring...",
"Ugh.",
"Cool! Keep up the good work!"
);
chat('April', @response);
sub chat {
my ($my_name, @responses) = @_;
print "Hi, my name is $my_name! What's yours? ";
chomp(my $name = <STDIN>);
print "Hi $name, nice to meet you! What are you up to? ";
my $response = '';
until ($response eq 'nothing') {
chomp($response = <STDIN>);
print $responses[rand @responses], "\n";
print "Anything else? ";
}
print "Goodbye, $name!\n";
}
matra555: That was a fun little snippet you posted. In response to your query, I went ahead and made a few quick changes to reflect something closer to how I would write this.
- Fixed indenting. I don't know if you just lost your indenting when you put that on your scratchpad or if you didn't indent, but it's very important. It's extremely difficult to see the scope of code without this.
- Both $name and $upto are declared inside of the subroutine (and $upto is renamed to a more appropriate sounding variable name). Whenever possible, you want your variables to have the smallest possible scope. This makes it less likely that you will accidentally reuse the variable at some other time and have to figure out why the value has changed.
- @reponse is now an argument to the subroutine. Now the &april subroutine does not depend on any variables declared outside of itself. Some people call these "pure" subroutines or functions. By doing this, you can now more easily reuse the subroutine. This brings me to my final point:
- While this is probably far more information than you wanted (or expected :), I went ahead and changed the name of the subroutine to &chat and made the first argument the name of whomever was chatting. Now, you can pass in a different name and a different set of responses and reuse this code all day long. Plus, that makes the subroutine call read like what it's doing: chat($person,@responses);.
I was glad to see you using strict and warnings. Failure to use those are two of the biggest mistakes new Perl programmers make. Your overall code was reasonable and easy to read and you generally had well chosen names. This is good and bodes well for your future in Perl :)
Cheers,
Ovid
OK, I know this place is about Perl, but I suspect a few people here are familiar with Scheme. Can anyone tell me why I get the following problem with mit-scheme?
1 ]=> (load "test.scm")
;Loading "test.scm" -- done
;Value: sqrt
1 ]=> (sqrt 4)
;Aborting!: maximum recursion depth exceeded
And test.scm looks like this:
(define (new-if predicate then-clause else-clause)
(cond (predicate then-clause)
(else else-clause)))
(define (sqrt x)
(define (sqrt-iter guess x)
(new-if (good-enough? guess x)
guess
(sqrt-iter (improve guess x) x)))
(define (improve guess x)
(average guess (/ x guess)))
(define (average x y)
(/ (+ x y) 2))
(define (good-enough? guess x)
(< (abs (- (square guess) x)) 0.001))
(sqrt-iter 1.0 x))
That works find if the sqrt definition has new-if changed to if. If you're curious, I got the problem from Section 1.1.7 of the Wizard book.
|