Re: error code from delete or beforeDelete?

On 30/05/12 14:54, AD7six wrote:

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. 

Totally agree, bad wording on my part. What I meant by one hit was one test case rather than a test method. The code being discussed ( a controller calling a method in a model and handling exceptions ) implied that it would be tested in 2 places. Once for the model test case in the raising of the correct exception and once in the controller for handling of the exception.

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.

Agreed. But it's really a criticism of how PHP currently handles exceptions rather than phpunit. Implementing a throws clause to a function would greatly enhance things if it became a syntax error to either not catch exceptions or filter them through... for example

public function a() throws AException { throw new AException(); }

// Declared correctly as a() throws an AException which is passed up the chain
public function b() throws AException, BException {
    a();
    throw new BException();
}

// Declared correctly as the AException does not get passed up but B and C do.
public function c() throws BException, CException {
    try{
        b();
    }
    catch( AException $a ){
        throw new CException();
    }
}

// Syntax error since BException is not handled and not declared.
public function d() throws CException {
    c();
}

At the moment, there is no way of knowing what possible exceptions can be thrown by any given function. Calling Model::delete(); could generate a whole list of them and without knowing the core source code very well, you can not tell from the function prototype. This makes it hard to write good tests IMHO and I would guess could also be a problem for tests when Exceptions are added or removed from the core in different Cake versions.
   

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.

Totally agree again, I am in no way saying that exceptions are not necessary or should not be used, I was really saying that they do not really fit in the beforeDelete() instance since data integrity 'could' already have been damaged by the time the exception is thrown.

Steve

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