[Catalyst] sample code for feedback

[Perrin Harkins]

On Thu, 2005-06-23 at 17:05 -0400, Rob Kinyon wrote:
> The things I would emphasize for Catalyst are the following:
> 1) It does all the mod_rewrite/C::A::P::Dispatch crap for you with the
> attributes. Given that, it's very easy to see immediately what's going
> to be the initial handler for what.

I think it's very flexible, but potentially quite confusing, and you
could easily create a situation where you would have trouble determining
which class handles which URL.  The advice I plan to give is for people
to choose a style (local, global, path, etc.) and stick with it.  The
debug output is also useful for figuring out where things are.

> 2) Each function is very small, often under 20 lines. Given that
> understandability is directly related to how many screens something
> covers, have a given method cover less than half a screen is
> important.

I don't think this is significantly different from other frameworks, but
we'll say what my code for the others looks like when I'm done with it.

> 3) Something I've started to do is to set $dbh =
> 'Poli::M::DB::Petition'. That way, I have less characters cluttering
> me up.

I don't like that kind of abbreviation, personally.  I would also never
call anything other than a database handle $dbh, but it could obviously
be something else.

> 4) I'd rewrite the sign() method to be:
> if ( $results->has_missing() or $results->has_invalid() ) {
>     # Handle error condition
> }
> 
> # Handle happyday

I'd have to put in a "return" after the initial forward, but I agree
that the conditional is easier to read.  I was trying to put the common
case first, but it makes more sense when you use the newer
Data::FormValidator which has a success() method.

> 5) I'd also emphasize how important $c->forward() is to this
> framework. CGI::App has it, but it's not central. In here, it's
> central to the whole way the thing works.

In terms of breaking your code into pieces, you mean?  I tend to write
things that way in CGI::App too, but the Catalyst docs encourage it
more, which is good.

> 6) Along those lines, I'd emphasize the fact that MyApp::V::TT is the
> place where the setup for the template handling goes. This means that
> your MyApp setup is for app-specific things, MyApp::C is for
> controller-specific things, etc. This means that you don't have to
> wade through irrelevant stuff to understand the specific configuration
> item(s) you're looking for.

It works to have one config file for all of them though, doesn't it?  I
haven't switched over to an external file yet, but I intend to.

- Perrin