TAP::Parser should be *easier* to subclass

[Steve Purkis]

On Jun 4, 2008, at 02:46, Eric Wilhelm wrote:

> # from Steve Purkis
> # on Tuesday 03 June 2008 14:38:
>
>> Take the
>> _initialize method: because I'd seen it in one place I assumed it was
>>   used everywhere.  It took me several minutes and the perl debugger
>> to realize it wasn't.  Being consistent could have saved me that time
>> and (ok, minor) frustration.
>
> Well, the '_' methods are supposed to be private.  Unfortunately, the
> documentation probably needs a better entry path and explanation of  
> how
> to customize/subclass things.

Hmm... I took them as being 'protected', hence fair game for  
subclasses.  But this is Perl:  unless you hide things through magic,  
nothing's really private :).

On init methods: IMHO, you should expect them to be overridden.   
That's what they're there for - doing custom initialization for your  
objects.


> Where we left this hanging (the last time this discussion came up) was
> that subclassing is not the way to customize the harness/parser/prove.

Well.. as a newcomer, that just doesn't make sense to me.  I think  
sub-classing is a natural way of customizing OO modules.  Really, I  
want the best of both worlds:

I like having configurable params like TAP::Harness' "formatter" &  
"formatter_class" params: so instance attrs like "perl_source_class"  
etc. in TAP::Parser, as you suggested.

I also like to be able to sub-class where it makes sense, and I  
expect not to have to rewrite a 500-line method changing only 1 or 2  
lines to do it.  That's where I think short methods like  
"make_perl_source" would make my life a bit easier.


> But, I don't think anybody has ever said what *is* the way, so the  
> lack
> of API stability/guidance/buy-in is (IMO) holding things back.

Agreed.

E.g.: In order to upgrade, my client needs the ability to use a  
custom Perl source, as they had with Test::Harness::Straps.  That I  
had to dive into the code base and patch things to achieve this is  
Not A Good Thing.  What's more, as it is not a community-accepted  
patch, it creates problems for my client - future versions of  
TAP::Parser will probably not be compatible with it, which means more  
maintenance work for them.

This will only deter people with customized setups from adopting  
TAP::Harness, which is a shame because it is a *big* improvement over  
Test::Harness!

I'd rather see this problem solved now, _with_ the buy-in of the tap- 
x dev community.  To that end, I'll put together a patch for  
TAP::Parser that:

1. uses TAP::Base throughout
2. uses instance attrs for class names
3. introduces 'make_<class>' methods
4. includes a short SUBLCASSING / CUSTOMIZING guide

Then we'll have something concrete to, discuss, adopt, reject, etc.


>> Of course it turns out there's already a TAP::Base class, it's just
>> not being used everywhere.  Is there any reason for that?
>
> Accident and/or lack of full-time architect.  Big changes tend to get
> warnocked or rejected for whatever reason.

That's either a bug or a feature of OSS development, depending on how  
you look at it. :)

-Steve