On Wednesday, 30 May 2012 15:00:08 UTC+2, Ratty wrote:
>
> 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
I agree that using exceptions for flow control (as gotos) is an awful habit
that deserves... punishment, I wouldn't say that justifies not using
exceptions where appropriate - I don't think the examples you're giving
are analogous.
a model test case can exercise all the different scenarios in one hit
>
>
A test method should test one thing only, writing multiple scenarios in one
test method is in and of itself a bad habit (of which I'm guilty of doing
in the past). Using exceptions or not does not IME make testing something
any harder or not - you either test that an expected exception has been
thrown or you check some return variable - the amount of work is the same
either way.
I get the feeling you're voicing an objection to how phpunit handles
exceptions with annotations rather than exceptions per-se, and you could if
you wish just wrap your tests in try/catch blocks and test the type of
exception thrown anyway.
Here's IMO a better example of when exceptions are appropraite:
try {
$result = $this->Foo->a();
} catch (Exception $e) { // It's an exception. *something* is wrong, it's
not necessarily important what - simply handle it gracefully and inform the
user.
$this->setFlash($e->message);
$this->redirect($this->referer());
}
/**
* Simulate a complex call to something. all functions in the same class
for brevity
*/
Class Foo {
function a() {
throw new AException("Some A exception");
...
return $this->b();
}
function b() {
throw new BException("Some B exception");
...
return $this->c();
}
function c() {
throw new CException("Some C exception");
...
return $this->d();
}
function d() {
throw new DException("Some D exception");
...
return $this->e();
}
function e() {
throw new DException("Some D exception");
...
return $result;
}
}
Instead of:
$result = $this->Foo->a();
if (!$result) {
$this->setFlash("Sorry. somewhere an error occurred");
$this->redirect($this->referer());
}
/**
* Simulate a complex call to something. all functions in the same class
for brevity
*/
Class Foo {
function a() {
if ("Some A exception") {
return false;
}
...
return $this->b();
}
function b() {
if ("Some B exception") {
return false;
}
...
return $this->c();
}
function c() {
if ("Some C exception") {
return false;
}
...
return $this->d();
}
function d() {
if ("Some D exception") {
return false;
}
...
return $this->e();
}
function e() {
if ("Some E exception") {
return false;
}
...
return $result;
}
}
With the latter - you either need to come up with a new return variable
format, stash an error message somewhere and retrieve it later - or simply
accept that you are blind to know where in the nested call things stopped
and returned false. It's also not easily possible to distinguish between a
"valid" false return value and some unexpected case. With an exception, you
can know as much or as little as you want about what happened by checking
the type of exception and/or the message.
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
