On 10/4/05, Dave Howorth <[EMAIL PROTECTED]> wrote:
> David Baird wrote:
>
> > I've done a bit of gentle refactoring in Maypole.pm, take a look and
> > see if it makes sense. Haven't changed any logic, just split some
> > stuff in handler_guts into separate methods, and changed any direct
> > hash accesses into proper method calls. Also, translated the return
> > code from is_applicable into a boolean, to remove one set of status
> > values from handler_guts. I put the view processing stanza into a
> > separate method and I'm intending to move that out of handler_guts()
> > and into handler(). Stop me if I start changing stuff you feel we
> > haven't reached a consensus on yet.
>
> Sorry, I have a number of questions about this.
>
> Let me start with a minor quibble. One of the things that I like about
> Maypole was that the code was maintained to a maximum width of 80
> columns. It suits the way I lay my screen out. Any chance we can
> continue this practice? It's a pain to have to resize my windows to a
> different size for different files.
Fair enough - and I've finally found the word wrap setting in Kdevelop!
> To an even more minor degree, I
> don't like all the extra blank lines - they mean I can't comprehend as
> much in a screenful. I can run perltidy so I can read the code but that
> doesn't fix comments.
I like to visually separate things that are not directly related:
$foo->squibble;
$foo->squabble;
$bar->frobnicate;
When that pushes a 'thing' (method or such) beyond a screenful, I take
it as a hint that the method is doing too much and needs to be split
up. A single screen of Maypole code tends to be more than I can
comprehend anyway! But I'll try and cut down on the newlines.
>
> Most importantly, I thought the plan was to to document and clarify
> first? I don't think we've done that, have we? Refactoring Maypole.pm
> isn't on the roadmap is it?
I want to spend some time writing docs, particularly for exception
handling and for controlling the workflow, but first, I need to have
code I can understand. I haven't changed how anything works (except
for the method attribute on handler).
>
> Somewhere in between in importance, I don't understand the purpose of
> all the __xxx methods. Since they're private, they can't be overridden,
> so what's their value? Especially since two are only one line long?
Nothing wrong with one-liner methods. You can spot their bugs a lot
easier. The big win is that it's easier to write unit tests for a
bunch of small methods, each of which has a single path from entry to
return, than for one large method, with multiple modes of operation.
So I'm trying to move code out of handler_guts. handler and
handler_guts are there to coordinate the workflow, I think the code
that actually implements each step should be outside these methods.
Then we'll have a clearer view of the different paths through them.
> What's the point in moving the view processing? I must be missing
> something again - perhaps the documentation?
We discussed an internal_redirect method, where it doesn't make sense
to process the view until the redirect has completed. Also, we agreed
that exception handling in the view should be different from that in
the previous steps. I'm not planning to dive in and start changing
that yet, but I wanted to put the view processing in a separate place
to facilitate that.
> I don't like the changes to call_authenticate. Changing $self to $r
> makes it less obvious what's happening, and Simon's comment was more
> helpful, IMHO.
OK, let's decide whether we call the thing $self or $r, because
currently, about half the time it's one and half the other. I think I
started calling it $r because it took me a while to realise $r and
$self were the same thing, and I wanted a visual reminder that the
invocant in plugins is the Maypole request object. If we have a
long-term view that we might separate the controller from the request,
then it should be called something else, i.e. $self, and I guess
that's what I'd prefer, as long as it's consistent.
Simon's comment didn't help me, but your explanation of it did,
unfortunately my translation of that into a comment is pretty poor.
I'll have another go.
d.
-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
Maypole-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/maypole-devel
