On Apr 3, 12:19 pm, Ben Bangert <[email protected]> wrote:
> The nice thing about keeping a controller small, is when it has a lot of
> actions. I have a few controllers in one of my projects that are very large.
> Though I consider this less than ideal and when I have some time I'll likely
> go in and refactor to break it down a bit. I guess the issue is that while a
> controller might start with 3 actions, it could grow... and if you didn't
> keep it brief to begin with who knows when you might next have some time to
> refactor it.
>
> I think your thought about keeping it to 20 or less lines of code is a pretty
> good rule of thumb, though sometimes for a big action, I can easily have
> 10-15 lines merely of assigning 'c' variables, so that prolly shouldn't be a
> hard limit. :)
I have one controller with 21 actions, most of which are a form/
formsave, but, some of the formsave actions do 8-10 'tasks' before
rendering the response. I thought consolidating tasks into an
external class/function would lend to better readability, but, each
action isn't duplicated, so, I'm just moving code from one place to
another. I think IBM once stated that 23 lines of code was the limit
for bugfree code which sort of corresponds to a single screen and I've
used that as a guideline. Converting 90k lines of php without a
framework to Pylons is allowing me to refactor quite a bit but, I was
just wondering if my approach was a bit extreme.
I'll have something like:
@validate(dadv_form, error_handler='add')
def addsave(self):
user =
meta.Session.query(users).filter(users.user_id==request.params['user_id']).one()
user.newval = request.params['newval']
user_log = user_log(somefunction,'C',user)
user.user_log.append(user_log)
secondtable =
meta.Session.query(blah),filter(blah.comment_id==request.params['comment_id']).one()
secondtable.comment = whatever
meta.Session.flush()
(task_id,status) = task().bigtask().add(get_client_id(), \
request.params['device_id'], \
request.params['username'], \
request.params['ip'], \
request.params['email'], \
request.params['path'], \
request.params['include'])
h.flash("whatever updated. Task %d, status %d" %
(task_id,status))
redirect('/blah/add')
While that isn't typical of every action, there are some actions that
require a lot more processing. Moving that out of the controller
would make the action something like:
@validate(dadv_form, error_handler='add')
def addsave(self):
(task_id,status) = do_addsave(request.params)
h.flash("whatever updated. Task %d, status %d" %
(task_id,status))
redirect('/blah/add')
It is certainly more compact, but, is it really any more readable? I
had been using sprox in TurboGears, but, had less success with it in
Pylons and haven't tried it recently, but, it would certainly change a
bit of the code. Replacing a number of lines of code with something
like:
provider.add(users, values=request.params)
or
provider.edit(users, values=request.params)
is certainly cleaner and provides a bit of readability.
The issue I'm facing is, I'm converting 90k lines of spaghetti code to
Pylons and looking towards the future from people that have maintained
large webapps. Just having a decent forms library (ToscaWidgets) has
made things a lot easier, though, I do miss ad_form from openacs. :)
I do appreciate the input. At least I know I'm not too far off base.
--
You received this message because you are subscribed to the Google Groups
"pylons-discuss" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/pylons-discuss?hl=en.
