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