On Mon, 2003-02-10 at 16:42, Stas Bekman wrote:
> Nathan Byrd wrote:
> > Hi all,
> > 
> > Below is an initial version of a patch against the latest CVS version of
> > RegistryCooker.pm (and RegistryLoader.pm) to support better subclassing,
> > including the ability to access module data from RegistryCooker in a
> > clean way and to add private data to subclasses, without relying on the
> > underlying array implementation (below my patch is the original message
> > written to the mod_perl list and Stas Bekman's response to provide the
> > context for this.)
> > 
> > The biggest part of this patch is from a change to the constants in
> > RegistryCooker to be _REQ instead of REQ, etc, because I couldn't have
> > two subs with the same name, and these constants are really internal
> > data anyway, especially if you want to be able to not rely on
> > RegistryCooker being implemented as an array.  Let me know if this is a
> > problem, I can also do it the other way around (another possibility
> > would be to change them instead to something like REQ_IDX or something,
> > but its kinda wordy).
> > 
> > To use this in a subclass, I call "public" with this names of the
> > variables I want to use for my subclass, then use them like normal: eg:
> > 
> > public qw(
> >     PAR_MEMBER
> >     ...
> > );
> > ...
> > $self->PAR_MEMBER(<value>); # Set a value
> > ...
> > $something = $self->PAR_MEMBER; #Get a value
> > 
> > We can also access the base class data in the same fashion:
> > 
> > my $r = $self->REQ;
> > 
> > 
> > I left the original constant usage inside RegistryCooker for performance
> > (no need to do an extra sub call inside RegistryCooker itself).
> > 
> > Please take a look at this and let me know what you think - I've tested
> > it under my configuration with the latest CVS and a now much nicer
> > looking version :-) of my Apache::PAR module as well as some simple
> > Registry scripts.  If this patch is acceptable I'll also send another
> > patch to add the appropriate tests to the test suite and add a section
> > about subclassing RegistryCooker to the porting guidelines and
> > RegistryCooker docs.
> 
> Good work, Nathan!
> 
> The patch needs some minor tweaking to conform with our coding style, but 
> overall it looks good. Though before we move on with this, won't it be simpler 
> to use the 'fields' pragma?
> 

Sorry about that - I'll take another look at the mod_perl coding
guidelines before submitting another patch.

Using fields would definitely be a good way of going about it.  I didn't
choose that approach in the code I submitted because:
1) I felt that implementing RegistryCooker as an array based object was
a good design philosophy, and my changes (hopefully) shouldn't have much
of an impact on the current working of the module unless its used in a
subclass (and we have a win in that situation.)
2) Anything that looks like pseudo-hashes scares me. :-)  Pseudo-hashes
seem to be in flux, and as a result I went with the more "old fashioned"
approach.

I can also see it either way though:
1) It would be less code to implement this change as a fields pragma.
2) The fields pragma is promised to work as advertised even when the
current pseudo-hash implementation goes away (as per the perlref doco.)

I think overall I currently feel more comfortable with the array based
patch mostly because even though its more lines, it seems to remain more
familiarity to the original code.  Also I felt that any performance or
integration testing that has already been done with Registry scripts may
be slightly invalidated with a change to the implementation structure of
the module.

While I was researching your question I mostly coded a solution to make
this change with the fields pragma, just haven't tested it yet.  If you
want, I can also post that version to compare the current proposed patch
with.

Thanks,

-- 
Nathan Byrd <[EMAIL PROTECTED]>

> 
> __________________________________________________________________
> Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
> http://stason.org/     mod_perl Guide ---> http://perl.apache.org
> mailto:[EMAIL PROTECTED] http://use.perl.org http://apacheweek.com
> http://modperlbook.org http://apache.org   http://ticketmaster.com
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [EMAIL PROTECTED]
> For additional commands, e-mail: [EMAIL PROTECTED]
-- 
Nathan Byrd <[EMAIL PROTECTED]>


---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to