[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