On Wednesday, 30 May 2012 13:00:47 UTC+2, Ratty wrote:
>
> Thanks for this thread actually Bill, it has given me a lot of pause for
> thought...
>
> Exceptions are a method of reporting WHAT has gone wrong or is about to go
> wrong. It is not a method of reporting WHERE it went wrong. In my
> experience they are primarily used for detecting "This should not happen"
> conditions. In your example the Category model delete function should not
> be called unless that category is empty, so an exception could be thrown if
> that condition is not met. This is a long way away from just calling
> delete() and see if it works. Remember that an exception can be thrown in
> beforeDelete(), delete(), afterDelete() in your model, any parent model, or
> any related model that is affected in HABTM, HasMany, ... relationships and
> they will all arrive at your catch block. You could end up with code like...
>
> try {
> $this->Category->delete( $id );
> } catch( NotEmptyCategoryException $nece ) {
> ....
> } catch( CategoryHasChildCategoriesException $ccce ) {
> ....
> } catch( CategoryReferencedInBlogTableException $crbe ) {
> ....
> } catch( ChildCategoryOfThisCategoryNotEmptyException $damn ) {
> ....
> }
>
> This logic, to my mind, would be far better positioned in the model
> function...
>
> Category::isDeletable($id) {
> $result = true;
> if( $this->hasItems() ) {
> $this->log( 'Cannot delete category containing items.', 'debug' );
> $result = false;
> }
> if( $this->hasChildCategories() ) {
> $this->log('Cannot delete category with child categories.',
> 'debug' );
> $result = false;
> }
> ...
> return $result;
> }
>
> CategoriesController::delete( $id ) {
> try {
> if( $this->Category->isDeletable($id) ) {
> $this->Category->delete($id);
> }
> }
> catch( Exception $e ){
> // If isDeletable has done it's job, this should never happen.
> $this->log( $e->message(), 'error' );
> ....
> }
> }
>
> Not only is this cleaner, but the model is FAR easier to write test cases
> for.
>
In what way is that FAR easier to test.
Note you've changed:
$this->Category->delete( $id );
into
if ($this->Category->isDeletable($id)) {
$this->Category->delete( $id );
}
Which means you're either checking things twice - or delete is nolonger
atomic.
AD
--
Our newest site for the community: CakePHP Video Tutorials
http://tv.cakephp.org
Check out the new CakePHP Questions site http://ask.cakephp.org and help others
with their CakePHP related questions.
To unsubscribe from this group, send email to
[email protected] For more options, visit this group at
http://groups.google.com/group/cake-php
