Kevin Dangoor wrote:
> You're correct that the automatic JSON may expose information that you
> don't want to expose. ... Elvelind fixed this for 0.9.
That's amazing--fixed in less than 12 hours after my review was posted!
I'll update the review tonight.
> As for the raise HTTPRedirect construct
>
> I'd also be fine with
>
> return cherrypy.HTTPRedirect(...)
Yes, I like that too.
> The nice thing about raise is something like this:
>
> @turbogears.expose()
> def mypublicmethod(self, arg):
> someval = self.dosomething(arg)
> return self.process(someval)
>
> def dosomething(self, arg):
> # oops, something isn't in the right state...
> raise cherrypy.HTTPRedirect(...)
>
> Of course, that is an exceptional condition, so maybe that makes perfect
> sense.
Let's think about what the "dosomething" method is doing here--it's
creating something to be processed, or short-circuiting to a redirect.
IMHO that type of control flow leads to buggy apps because you're never
quite sure what to expect when you call a method--the control flow is
not explicit (very bad in a complex app). It would be more clear to do
it this way:
@turbogears.expose()
def mypublicmethod(self, arg):
try:
someval = self.dosomething(arg)
except InconsistentStateError:
return cherrypy.HTTPRedirect(...)
return self.process(someval)
def dosomething(self, arg):
# oops, something isn't in the right state...
raise InconsistentStateError()
Again, I think it's important to keep application flow clear and
explicit.
