On Sunday 20 Dec 2009 21:15:53 Parag Kalra wrote:
> Hello All,
>
> Just started learning OOPs in Perl from the book 'Intermediate Perl'
> written by Randal.
As well as brian d foy and Tom Phoenix I may add.
>
> I have written a small program given below. Its working at most places
> except while printing color of unnamed creatures. Just execute the script
> and you will figure out the bug. Could someone explain where am I going
> wrong in the following script. (I know I have not used strict pragmas. :) )
>
I've ran it and I still cannot figure out the bug. Why didn't you use "strict"
and "warning"? And here are some more comments about the code, one of which
may fix the bug.
> TIA
>
> Cheers,
> Parag
>
> #!/usr/bin/perl
> ####################
> #Author: Parag Kalra
> ####################
> { package Animal;
> sub speak {
> $class = shift;
> print $class->name, " goes ", $class->sound, "\n";
> }
1. Generally, every subroutine (or in this case method) should have an
explicit return. Use an empty return (which returns a false value in both
scalar and list contexts) if you don't have anything to return, and want it to
be a procedure.
<<<<
return;
}
>>>>
2. "$class = shift;" should better be "my $self = shift;". You need the "my"
keyword to localise the variable to a block (because you're also assigning to
it in other places. Please call the object (or class *instance*) "$self"
instead of "$class", if you don't have a better name to call it. If it's
called "$class" then people may be misled to believe it is a class name.
You may also wish to write "my $animal = shift;" instead if you feel like
being more specific. I admit that I normally default on using "my $self =
shift;"
>
> sub eat {
> $class = shift;
> $food = shift;
> print $class->name, " eats ",$food, "\n";
> }
You should add my's , an explicit return, and make sure a space follows the
second comma.
>
> sub color {
> $class = shift;
> print $class->name, " has ", ${$class}{Color}, " color\n";
> }
$self->{Color} is preferable over ${$class}{Color}. (Easier to understand) An
an English nit - 'it's has a red color" instead of "has red color".
>
> sub name {
> $class = shift;
> ref($class) ? (${$class}{'Name'}) : ("An unamed $class");
> }
Why are you writing a method that acts on both an object reference and a class
name?
>
> sub new {
> $class = shift;
> $name = shift;
> $self = {Name=>$name, Color=>$class->default_color};
> bless $self, $class;
> }
>
Again - an explicit return.
Finally there is no good reason to put the entire package under a block.
> }
>
> { package Horse;
> @ISA = qw(Animal);
"use base" or "use parent" are preferable over setting @ISA explicitly. Unless
you're using Moose of course.
> sub sound {qw(Neigh)};
> sub default_color{'Brown'};
> }
>
> { package Sheep;
> @ISA = qw(Animal);
> sub sound {qw(Baaah)};
> sub default_color{'White'};
> }
>
> $horse = Horse->new('Chetak');
> $horse->speak;
> $horse->eat('Long grass');
> $horse->color;
>
I would prefer an explicit "package main;" statement here.
> Horse->new();
> Horse->speak;
> Horse->eat('Long grass');
> Horse->color;
>
> $sheep = Sheep->new('Chimpu');
> $sheep->speak;
> $sheep->eat('Small grass');
> $sheep->color;
>
> Sheep->new();
> Sheep->speak;
> Sheep->eat('Small grass');
> Sheep->color;
>
Regards,
Shlomi Fish
--
-----------------------------------------------------------------
Shlomi Fish http://www.shlomifish.org/
The Case for File Swapping - http://shlom.in/file-swap
Bzr is slower than Subversion in combination with Sourceforge.
( By: http://dazjorz.com/ )
--
To unsubscribe, e-mail: beginners-unsubscr...@perl.org
For additional commands, e-mail: beginners-h...@perl.org
http://learn.perl.org/
