Wednesday, May 30, 2012

Re: error code from delete or beforeDelete?

It's far easier to test since all the logic is in the model and a model
test case can exercise all the different scenarios in one hit.

The test with exceptions would need to be something like...

function testCategoryNotEmptyDeletion {
... setup illegal data to throw the right exception but none of the
others ...
try {
$this->Category->delete();
$this->fail('Exception expected');
}
catch( CategoryNotEmptyException $cnee ) {
// Pass, This one was expected
}
catch( Exception $e ) {
$this->fail('Unexpected Exception');
}
}

for each exception that can be thrown making the way they are thrown and
the order they are thrown in by beforeDelete() very important.

say, for example the model had

CategoryModel::beforeDelete() {
delete or move some related data to allow category deletion
parent::beforeDelete();
}

MyCategoryModel::beforeDelete() {
... detect error condition ...
throw new ErrorConditionException('Error detected');
parent::beforeDelete();
}

this could be coded as :

CategoryModel::beforeDelete() {
delete or move some related data to allow category deletion
parent::beforeDelete();
}

MyCategoryModel::beforeDelete() {
parent::beforeDelete();
... detect error condition ...
throw new ErrorConditionException('Error detected');
}

Which would still pass the test as the exception can be thrown, but the
parent has already modified some data.


I totally agree that you could end up checking data twice, but only if
you do all the same checks in beforeDelete() as you do in isDeletable().

There are some scenarios that should only be possible from the UI like
trying to delete a category while it still has items in it. Categories
deleted by code would need to remove the items from the category before
deleting it and should have a test case to ensure that this happens.
beforeDelete() would then have no need to check that condition since it
would have been checked by isDeletable().


I have inherited some code that basically used Exceptions as a form of
flow control and believe me, debugging it and understanding it was a
nightmare. Admittedly languages like Python rely on them for exactly
that... but I am still not sure that is a good thing.

This is all just my opinion of course !!!!

Steve (Ratty)



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
> cake-php+unsubscribe@googlegroups.com For more options, visit this
> group at http://groups.google.com/group/cake-php

--
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
cake-php+unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/cake-php

No comments: