[RFC] Text::Echelon v0.01

[Russell Matbouli]

On Sun, Dec 16, 2001 at 01:56:04PM +0000, Tony Bowden wrote:
> Looks nice!

Thank you. I just wanted to get something out, then fix it up when
people commented on it (as you helpfully have done). I've been fixing up
the pod and have added to it since then. I might release 0.02 by the 
time I've finished tech support today.

> 1) I'd probably change some of the function names:  get doesn't really
>    tell me what it's getting. Do these all even need to be public?
<snip>

Yes. This would be much better. I will probably do it this way when I
figure out how (UTVi have copies of "The Perl Cookbook" and "Advanced
Perl Programming", so I'm sure I'll figure out how soon enough).

> 2) You're using @wordlist in a strange way: every time you call new()
>    you're setting a package global. You should either make this instance
>    variable, which gets read every time (if you're going to
>    allow instances of your class to change them in any way), or, probably,
>    class data that only gets set once.

What should I do with @wordlist to make it an instance variable? Isn't
it okay that this gets populated when new() is called...?

>    To be honest, I'm not really sure why you've written it in an pseudo-OO
>    style at all... 

Pseudo-OO is what I know... The thing about @wordlist makes more sense
now. Do away with the need to call new() and just have it populate when
it is first called...? Would I just do something similar in a BEGIN
block?

> 3) I'd drop the 'use warnings'  - I don't see any other 5.6-isms, so no
>    point in breaking backwards compatability...

Fair enough. I meant to remove that before releasing it :)

> 3b) The srand() is the other lower-edge limit version of this. You haven't
>     needed the explicit srand for quite some time...

Cargo-cultery. How should I do it? Just call rand without doing srand
beforehand?

> 4) while (<DATA>) { @wordlist = <DATA>; } 
>    probably isn't doing what you think it's doing...
>    Or maybe it is, but not the way you'd like it to.
>    Or something.

All I wanted was to make the huge list of phrases useable. My thought
was to throw it into an array. It looked messy, so I put it in __DATA__

Care to explain...?

> 5)
>    This is probably better, if I read it correctly, as:
<snip>

Mmmmm, looks much better. I'll use that, cheers :)

> 6) I'd suggest moving the test to Test::Simple! Saves mucking about with
>    all that print OK / not OK stuff.

Yeah. It's what h2xs generated, and I thought I'd try it out without
R'ing any FMs. That's why it is stupid. I'll get a clue about testing
soon...

> Hope this helps. If you want a hand tidying any of this up more, just
> let me know!

Thanks! I'm aware that this module is ... alternately useful[1]... but I
wanted to write a module and get feedback to learn from it. It has been
good so far. Next step is generating suspicious natural language 
sentences :)

[1] euphemism
-- 
Russell Matbouli       |
russell at futureless.org |  IN THE GRIM FUTURE OF HELLO KITTY THERE IS ONLY WAR
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/e73810ec/attachment.bin