[RFC] Text::Echelon v0.01

Russell Matbouli russell-belfast-pm at futureless.org
Sun Dec 16 10:00:49 CST 2001


On Sun, Dec 16, 2001 at 03:13:29PM +0000, Andrew Wilson wrote:
> Mainly it doesn't work because you don't construct an object in your
> constructor.  If you fix that the rest of the code works although
> you do some things that I wouldn't do.

Thanks for pointing that out. It passes your tests now...

> 1) You construct the wordlist in your new function this would be better
> moved into the pacakge initialisation.

I was confused. I was thinking OO but not doing OO. I understand better
now, I hope.

> @wordlist = <DATA>;

Yes. I don't know how I got to the horrible code that was there before,
but it's gone now.

> 2) You might as well just chomp the list and do your srand
> initialisation at package level as well.  Wordlist is a package

I've got rid of the srand, as Tony pointed out that it wasn't needed.
Premature optimisation blah blah, but I was thinking that there's no
point in chomping hundreds of scalars when only a few are going to be
used...

> variable, standard perlstyle says it should have initial caps (I
> usually ignore this one, but you should be aware of what you're
> ignoring ;-)

Okay, I'm bad :) I'll try to follow perlstyle now.

> 3) You dont need a lot of the temporary variables that you scatter
> about your code.

*nod* This is because I pasted in most of the code from a script. I much
prefer how you recoded it, except for the implicit returns (my Java
weenieness).

> 4) You should try to learn how to use perl's list operators, I much
> prefer
> 
> join $delim, map $self->get (1 .. $num);

mmm, lovely... Going to learn the list ops now...

<snip nice explanation>

> 5) You keep redefining default values for things, then you pass in the
> default value.  You only really need to define them in the method that
> actually uses them and you don't need to pass in the default value.

Initially, I was just passing in those values, then I thought it'd be
better to give sensible defaults instead (but forgot not to pass them
in).

> I've modified the module and wriiten another test file for it, so you
> can see what I'm talking about.  As I've said before when you fix the
> constructor all the rest of it worked and these are jsut style points
> really.  If you want to discuss any more of it give me a shout

Thanks a lot! Much appreciated. It's not hard to tell my baby-perl from
your well written code :)
-- 
Russell Matbouli       |          Your legs are like threads of cotton,
russell at futureless.org |      though much thicker, and filled with weevils
PGP KeyID: 0x3CA84CF4  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 232 bytes
Desc: not available
Url : http://mail.pm.org/archives/belfast-pm/attachments/20011216/4c2731f0/attachment.bin


More information about the Belfast-pm mailing list