Re: Best practices in a thin-controller application

Hi Adam and others,

As far as I know, it's generally considered a good practice to throw 
exceptions anywhere in the code (constructor or otherwise) where an 
exceptional condition is discovered that you don't want the object to 
handle. Sometimes you have situations like __toString() where shouldn't 
throw exceptions, but those are pretty rare and usually documented.

If you decide that, in your application, you don't want any domain model 
objects to be constructed without a completely valid state, then using 
exceptions to enforce that architectural decision is a perfectly valid 
and effective thing to do. That would be an appropriate use of the 
language features. I should note that this is just one possible way to 
architect an application; IMHO, many architectural decisions like this 
are subjective, and as such, there is no universally accepted "right" 
way to do it. Point is, don't feel like you *have* to this way, and only 
this way. :)

BTW, in my opinion, it's good to send the correct HTTP status code with 
each response, which to me includes sending a 400 for requests that 
don't make sense, a 403 when they try to do something that's not 
allowed, a 500 for errors like uncaught exceptions, and so on. My 
rationale is that the exact meaning of the HTTP status codes should be 
decided by each application that speaks HTTP, to ensure it's 
communicating correctly to HTTP clients. If there's an easier way than 
what you're currently doing to achieve this, then I'm unaware of it.

Regards,
Bryce Lohr

Adam Jensen wrote:
> That's pretty much what I ended up doing.  As far as I understand it,
> it's OK to throw exceptions from the constructor when the object can't
> be constructed in a valid state.  If our Trip object absolutely must
> always have a valid ID property and the provided ID isn't valid, it's
> only natural to throw an exception; otherwise you end up creating an
> object whose internal state is invalid (which is one of the things
> Nino was warning us against).
>
> And you're absolutely right about the varying subclasses of exceptions
> for varying failure conditions...in my controller, it allows me to do
> something like this:
>
> [code]
> public function viewAction()
> {
>     try {
>         $this->view->trip = new Travel_Trip($this->_getParam('id', null));
>     } catch (Travel_Trip_Exception_MissingIdentifier $e) {
>         $this->_setParam('message', 'You must provide a trip ID.');
>         $this->_forward('bad-request', 'error', 'default');
>         return;
>     } catch (Travel_Trip_Exception_InvalidIdentifier $e) {
>         $this->_setParam('message', 'Invalid trip ID!');
>         $this->_forward('bad-request', 'error', 'default');
>         return;
>     } catch (Travel_Trip_Exception_NotFound $e) {
>         $this->_forward('not-found', 'error', 'default');
>         return;
>     }
> }
> [/code]
>
> Of course, this can get pretty complicated if you're really specific
> about the failure conditions...so for my own sanity's sake, I've tried
> to leave it limited to the above.
>
> (Off topic...my "bad-request" action [in the ErrorController] is used
> for situations like this where the user's request is invalid for some
> reason.  As the name implies, it outputs an HTTP 400 "Bad Request"
> header along with whatever message is assigned to the "message"
> parameter.  I've been wondering now for some time: (1) is this an
> appropriate use of the HTTP 400 header, and (2) does the Zend
> Framework provide a better way of doing this?)
>
> Thanks!
> Adam Jensen
>
>
> On Tue, Jul 22, 2008 at 10:08 AM, Bryce Lohr 
> <brycel-kr8tFwxIqV2MVkprmlnVt0B+6BGkLq7r@xxxxxxxxxxxxxxxx> wrote:
>   
> > Hi David,
> >
> > This would be a perfectly valid thing to do, except it would be better if
> > you had specific exception sublcasses, such as "MissingIdException" and
> > "IdNotFoundException" or similar. The idea to ensure that the class name,
> > rather than the message, differentiates the exceptions so the code down the
> > call stack can correctly decide how to handle any caught exception.
> >
> > Regards,
> > Bryce Lohr
> >
> > David Mintz wrote:
> >
> > On Sat, Jul 19, 2008 at 1:35 AM, Adam Jensen 
> > <jazzslider-Re5JQEeQqe8AvxtiuMwx3w@xxxxxxxxxxxxxxxx> wrote:
> >     
> > > <snip/>
> > >
> > > OK...so if the object can't be constructed valid, it shouldn't be
> > > constructed at all.  As far as I know, the only way to do that in PHP
> > > is with exceptions, as per my first example.  And then I can use
> > > multiple exception subclasses to handle the various reasons why the
> > > construction failed, which the controller can use to provide the user
> > > with appropriate error messages.
> > >
> > >       
> > So -- would you advocate, e.g., a model constructor roughly like
> >
> > function __construct($id = null) {
> >
> >    if (! $id) { throw new Exception("missing id parameter"); }
> >
> >    if (!$this->isValidId($id)) {throw new Exception("invalid id"); }
> >
> >    // instantiate a subclass of Zend_Db_Table_Abstract $this->someTable
> >
> >    if (! $object = $this->someTable->find($id)) {
> >
> >               throw new Exception("record $id not found"); }
> >
> >   // continue initializing etc
> >
> > }
> >
> > I seem to recall reading somewhere that throwing exceptions in a constructor
> > is bad form but I forget where or why.
> >
> >
> >
> > --
> > David Mintz
> > http://davidmintz.org/
> >
> > The subtle source is clear and bright
> > The tributary streams flow through the darkness
> >