Re: splitting &preparse

At 09:42 04/09/22 +0200, Terje Bless wrote:

>-----BEGIN PGP SIGNED MESSAGE-----
>Hash: SHA1
>
>Martin Duerst <duerst@w3.org> wrote:
>
> >I just did a few CVS commits to split &preparse into two separate
> >subroutines (now that it's called twice): [...]
> >If this was a really bad idea [...] please tell me.
>
>[/me puts on the nitpicking hat]

Thanks, appreciated!


>Well, I actually think it's a bad idea (otherwise it would probably have been
>that way already), because now we have two subroutines that in the abstract
>perform the exact same function. However, I do consider the tradeoff you've
>made -- i.e. the sub performs two unrelated functions so it should be
>split/separated -- an equally valid one (just one I happen to disagree with).

I think in terms of functionality, we could go either way.
I wouldn't have done the split if you hadn't done the second
call, because I was affraid calling &preparse is a resource hog.
The reason I have done the split is that I think &preparse_meta
will end up in the charset detection/conversion module, but
&preparse_doctype won't, so the split helps sort out the
functionality.


>IOW, no I don't think this change was the best design for it, but that's just
>my opinion; and I think we should go by your opinion on this issue. :-)

Thanks!


>I'll try to take a look at that code to see if they can be further simplified,
>but I think Ville is more familiar with HTML::Parser so he's probably the one
>to spot any issues.

Looking forward to comments from Ville, then.


>Initial thought: can we abstract this out further so that there is a
>&preparse() that takes a sub-ref as an argument and &preparse_foo() both call
>this sub with the necessary code ref for their specific function?

We probably could, but that would mean a lot of sophistication
for not too much gain, and it would tie the two functions to a
third rather than allowing them to live separately e.g. in different
modules.

Regards,    Martin.

Received on Wednesday, 22 September 2004 09:29:50 UTC