TAP::Parser should be *easier* to subclass [PATCH]

[Steve Purkis]

> On Jun 7, 2008, at 18:11, Eric Wilhelm wrote:
>> # from Steve Purkis
>> # on Saturday 07 June 2008 07:42:

Hi all,

You'll find a patch against 3.10 attached.  Apologies: I didn't patch  
trunk because I wanted this to work against 3.10 (so my client  
doesn't have to wait around for the next release, in case the patch /  
general idea is accepted).

It ended up being more work than I'd thought (surprise :))...  See  
below for a summary, look forward to feedback.


So here's what's in it:

>> 1. uses TAP::Base throughout
>
> Ok, but I do suggest caution.  Adding consistency is good, but my main
> concern is that we may end up with a "foolish consistency" where the
> base is being pulled too many different directions.

1. Introduced TAP::Object.

TAP::Base was too specific in what it did to serve as a base class  
for *all* TAP::* objects.  Eric: Hopefully this isn't foolish  
consistency.  It's a *really* basic class, which should avoid the  
problem of being pulled in too many directions.

Aside: I would've preferred moving functionality out of TAP::Base to  
somewhere like TAP::Base::Callbacks, but I got scared I'd break  
random things so decided against it.


>> 2. uses instance attrs for class names
>
> Yes, with mutators.  Apply liberally.

2. Uses instance attrs + mutators for class names

Added accessors to TAP::Parser:
+        source_class
+        perl_source_class
+        grammar_class
+        iterator_factory_class
+        result_factory_class

Defaults are now override-able methods.

Not applied liberally enough for my liking - I had hopes to do this  
in classes like TAP::Parser::Source, but I'm outta time unfortunately.


>> 3. introduces 'make_<class>' methods
>
> I take it that this acts as a point to override/inject options into  
> the
> sub-object's constructor call?

3. make_* methods

Eric: spot on.

The parser now does this:
+sub make_source      { shift->source_class->new(@_); }
+sub make_perl_source { shift->perl_source_class->new(@_); }
+sub make_grammar     { shift->grammar_class->new(@_); }
+sub make_iterator    { shift->iterator_factory_class->new(@_); }
+sub make_result      { shift->result_factory_class->new(@_); }

In one of the tests I've done this:
sub make_source      { shift->SUPER::make_source(@_)->custom }

Though you could do whatever you like.  I know some people don't like  
1-line methods, but strategically placed, they can make subclassing a  
breeze.


4. Parser is now the central 'maker'

This was required in order to make the supporting classes respect the  
user's customizations & overrides above.  I had to changed a few  
constructor sigs to achieve this, namely TAP::Parser::Grammar &  
TAP::Parser::Source.  They now take hashrefs.

Note: I'm concerned about circular refs here... don't suppose  
Scalar::Util's weaken is fair game from a backwards compat POV?


5. ResultFactory & IteratorFactory

Split both TAP::Parser::Result & TAP::Parser::Iterator into 2  
distinct classes: base class and factory class.  This should make  
them easier to subclass.

I also added register_type() to ResultFactory, so you don't need to  
subclass if you just want to add a custom type or change an existing  
one.


6. All tested, of course

There could stand to be some more tests for subclassing, but (a)  
that's always the case, and (b) I'm outta time :-/


>> 4. includes a short SUBLCASSING / CUSTOMIZING guide
>
> Yes!  I suggest TAP/Harness/Customizing.pod.

7. API docs updated

I've tried to keep the POD up-to-date as I go.  I've left off the guide.

Basically, I came to the conculsion that there's not much sense  
writing anything until the code is agreed on.

Also: I want to set people's expectations here.  I only intended to  
write a small SUBCLASSING / CUSTOMIZING section in TAP::Parser or  
something, not write a book on it.  I'm not sure if I'll have time to  
cover anything other than TAP::Parser.


> The HACKING.pod is mostly my fault, but I may be the only person who
> ever made edits to it.  Please read carefully and with a grain of  
> salt.
> We should probably add links in there, as well as notes on the  
> internal
> details of the customization support and some cleanup.

Thanks - I did read through (& update) part of this...

Incidentally, I've tried running perltidy on some things, and found  
it changed things that I hadn't touched at all.  In the interests of  
the patch being readable I decided not to run it after that...  Can  
easily be run in the future.


> For quick 'WTF?' questions, you might find Andy or me in #toolchain on
> irc.perl.org.


Tried pinging you guys, but it is Sunday after all ;-).

Unfortunately IRC's off-limits while I'm onsite :-/

Cheers,
-Steve

-------------- next part --------------
A non-text attachment was scrubbed...
Name: Test-Harness-3.10-easier-subclassing-1.patch
Type: application/octet-stream
Size: 52775 bytes
Desc: not available
Url : http://www.hexten.net/pipermail/tapx-dev/attachments/20080608/97c8cb4f/attachment-0001.obj 
-------------- next part --------------