SitePoint Sponsor

User Tag List

Results 1 to 23 of 23
  1. #1
    SitePoint Zealot mr_jeep's Avatar
    Join Date
    Feb 2004
    Location
    Canada
    Posts
    131
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    returning error object or true/false ???

    Hi, now I am confused.. very very very confused. I'm wondering if, in a method,
    I should return an error object (somewhat similair to Pear_Error) or just return true or false. I looked to some PEAR files and I can't find the right pattern about when returning error object or true/false. Here's an exemple :

    PHP Code:
        // }}}
        // {{{    open()
        
        /**
         * Ouvre le fichier
         *
         * @param mode int Le mode du fichier
         *
         * @return bool true si le fichier a été ouvert, false sinon
         *
         * @access public
         */
        
    function open($mode FILE_OPEN_RW_CREATE)
        {
            
    $this->fp = @fopen($this->file$mode);

            return (
    $this->fp) ? true false;
        } 
    or :

    PHP Code:
        // }}}
        // {{{    open()
        
        /**
         * Ouvre le fichier
         *
         * @param mode int Le mode du fichier
         *
         * @return mixed true si le fichier a été ouvert sinon un objet error
         *
         * @access public
         */
        
    function open($mode FILE_OPEN_RW_CREATE)
        {
            
    $this->fp = @fopen($this->file$mode);
            if (!
    $this->fp) {
                return 
    raiseError(sprintf('Unable to open file %s'$this->file));
            }
            
            return 
    true;
        } 

  2. #2
    Non-Member
    Join Date
    Jan 2004
    Location
    Planet Earth
    Posts
    1,764
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Personally I would return a boolean, but that is just convienence for me

    In saying that, there are certain situations where you are better to return an object though, even I might add, you need to return an object

  3. #3
    SitePoint Zealot David C's Avatar
    Join Date
    Nov 2003
    Location
    New York!
    Posts
    105
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    I think code is much easier to follow if you're returning boolean values - I would be shocked if isValid() didn't return true or false.

    Also, by separating external classes from each of your classes, you can make your code much more portable.

  4. #4
    simple tester McGruff's Avatar
    Join Date
    Sep 2003
    Location
    Glasgow
    Posts
    1,690
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    I would think it's always better to keep things as simple as possible.

  5. #5
    eschew sesquipedalians silver trophy sweatje's Avatar
    Join Date
    Jun 2003
    Location
    Iowa, USA
    Posts
    3,749
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    instead of
    Quote Originally Posted by mr_jeep
    PHP Code:
    function open($mode FILE_OPEN_RW_CREATE)
        {
            
    $this->fp = @fopen($this->file$mode);

            return (
    $this->fp) ? true false;
        } 
    you could take it all in one shot:
    PHP Code:
    function open($mode FILE_OPEN_RW_CREATE)
        {
            return (
    $this->fp = @fopen($this->file$mode)) ? true false;
        } 
    Jason Sweat ZCE - jsweat_php@yahoo.com
    Book: PHP Patterns
    Good Stuff: SimpleTest PHPUnit FireFox ADOdb YUI
    Detestable (adjective): software that isn't testable.

  6. #6
    SitePoint Guru
    Join Date
    Oct 2001
    Posts
    656
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Boy am I glad that PHP 5 features exceptions

  7. #7
    SitePoint Guru
    Join Date
    Dec 2003
    Location
    oz
    Posts
    819
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Yeh I know !!

    This proceedural error handling is a nightmare in comparison.

  8. #8
    SitePoint Enthusiast
    Join Date
    Feb 2004
    Location
    France
    Posts
    58
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    I personnaly like to standardize on "false" for error, anything else otherwise. This is what built-in PHP functions do most often anyway.

    Treating error as objects requires additional work in error checking. What I like to do is have a $error variable in all my object, which contain false if the function worked properly, or an error message otherwise (on top of returning false). That way if I want some detail about the error I can just check the message, and if I don't I just look at the return value.

  9. #9
    SitePoint Guru
    Join Date
    Dec 2003
    Location
    oz
    Posts
    819
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    OOP Exception handling takes care of exactly that and alot of other problems when dealing with errors.

  10. #10
    Non-Member coo_t2's Avatar
    Join Date
    Feb 2003
    Location
    Dog Street
    Posts
    1,819
    Mentioned
    1 Post(s)
    Tagged
    1 Thread(s)
    I've gotten so I return an error object in any situation where something failed or something didn't go as expected. I would consider fopen() failing as something going wrong, so I would return an error.

    Speaking of PHP5 exceptions. Will PHP's built-in functions throw exceptions?

    --ed

  11. #11
    SitePoint Addict
    Join Date
    Aug 2002
    Location
    Ottawa, Ontario, Canada
    Posts
    214
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by mr_jeep
    Hi, now I am confused.. very very very confused. I'm wondering if, in a method,
    I should return an error object (somewhat similair to Pear_Error) or just return true or false.
    I always return an AppError object whenever I encounter an error. For the simple reason that it removes ambiguity.

    For example, if you have a function that does a check say, user_exists() - it should return a true if a user exists, or a false if a user doesn't exist. Let's say in this function you query a db for this check. What if the query fails? Not that it returns 0 rows, which should be a false, but that there is a trappable db error? You can't return a false to indicate the error, since false is a valid response of the function [and thus the error should not be taken to mean the user doesn't exist because they might be in the db despite the error]. I would trap the db error, and create, then pass back an AppError object. I haven't abended my code, but I can now handle the error, and if I really want to, I could parse the error and set up different application responses depending on the type of error returned.

    Coding for returning errors by booleans leads to "coding by exception" - That is, "a false means error, except when...."

    Cheers,
    Keith.

  12. #12
    simple tester McGruff's Avatar
    Join Date
    Sep 2003
    Location
    Glasgow
    Posts
    1,690
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    It may be better not to do two things in one function. Kind of blurs the focus a little.

  13. #13
    SitePoint Addict
    Join Date
    Aug 2002
    Location
    Ottawa, Ontario, Canada
    Posts
    214
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by McGruff
    It may be better not to do two things in one function. Kind of blurs the focus a little.
    I think you are missing the point...

    Cheers,
    Keith.

  14. #14
    ********* Victim lastcraft's Avatar
    Join Date
    Apr 2003
    Location
    London
    Posts
    2,423
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    Hi...

    Quote Originally Posted by Taoism
    Coding for returning errors by booleans leads to "coding by exception" - That is, "a false means error, except when...."
    I don't think is solved by returning application objects either.

    I would expect methods named as assertions to return true/false, for example userExists(). A problem with returning an error object is that if it has data in it, such as an error message, it will evaluate true. So...
    PHP Code:
    if (! $authenticator->userExists()) {
        ...

    ...could cause surprises.

    I find that methods that fetch information are mor elikely candidates for error objects if they are high up in the application hierarchy. This is a no brainer if you are carrying a message that will be viewed by the user. You still have to check that the response is an error, though, unless using the null value pattern. Checking class names is restrictive so we tend to have both success and failure objects...
    PHP Code:
    class Success() {
        var 
    $_result;

        function 
    Success(&$result) {
            
    $this->_result = &$result;
        }
        function 
    isError() {
            return 
    false;
        }
        function &
    getResult() {
            return 
    $this->_result;
        }
    }
    class 
    Failure() {
        var 
    $_message;

        function 
    Failure($message) {
            
    $this->_message $message;
        }
        function 
    getError() {
            return 
    $this->_message;
        }
        function 
    isError() {
            return 
    true;
        }
    }
    class 
    DatabaseError extends Failure {
        function 
    DatabaseError() {
            
    $this->Failure('Please contact the system administrator...');
        }

    That way the conditional is simpler...
    PHP Code:
    $response = &$authenticator->findUserByLogin();
    if (
    $response->isError()) {
        ...
    }
    $permissions $authorisor->getPermissions($response->getResult()); 
    To be honest, at the lower levels this is overkill. Just adding an isError() and getError() to the finder (here the authenticator) is sufficient.

    yours, Marcus
    Marcus Baker
    Testing: SimpleTest, Cgreen, Fakemail
    Other: Phemto dependency injector
    Books: PHP in Action, 97 things

  15. #15
    SitePoint Addict
    Join Date
    Aug 2002
    Location
    Ottawa, Ontario, Canada
    Posts
    214
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by lastcraft
    Hi...
    I don't think is solved by returning application objects either.
    I respectfully disagree. It solves the issue completely.

    Quote Originally Posted by lastcraft
    A problem with returning an error object is that if it has data in it, such as an error message, it will evaluate true. So...
    PHP Code:
    if (! $authenticator->userExists()) {
        ...

    ...could cause surprises.

    Which is why I code the AppError object to contain an is_error() method:
    PHP Code:
    //inside AppError
      
    function is_error($obj){
        return (
    is_object($obj)&&is_a($obj,'apperror'))?true:false;
      } 
    Then I can handle an error check like so....
    PHP Code:
    if(AppError::is_error($result=function_or_object_method_call()){
      
    //handle error
    }
    //else clause is optional depending on how
    //error is handled 
    Problem solved. It works quite well and removes ambiguity.

    Cheers,
    Keith.

  16. #16
    ********* Victim lastcraft's Avatar
    Join Date
    Apr 2003
    Location
    London
    Posts
    2,423
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    Hi...

    Quote Originally Posted by Taoism
    Problem solved. It works quite well and removes ambiguity.
    I respectfully disagree with your respectful disagreement .

    Unfortunately by doing a class name check in the AppError you have tied youself to a specific class hierarchy. You have in effect turned PHP into a version of Java, but without interfaces to get you out of trouble. That was why I added the Success wrapper as well - to restore polymorphism.

    The hard coding of the class name has big and painful implicatons for other people extending your framework and for testing. Also it makes the is_error() function mysterious. Someone trying to figure out your system basically has to read the code, as it's invisible to the interfaces.

    Of course this abomination was originally perpetrated by PEAR and various authors have copied it by example .

    yours, Marcus
    Marcus Baker
    Testing: SimpleTest, Cgreen, Fakemail
    Other: Phemto dependency injector
    Books: PHP in Action, 97 things

  17. #17
    SitePoint Addict
    Join Date
    Aug 2002
    Location
    Ottawa, Ontario, Canada
    Posts
    214
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by lastcraft
    Unfortunately by doing a class name check in the AppError you have tied youself to a specific class hierarchy. You have in effect turned PHP into a version of Java, but without interfaces to get you out of trouble.
    I am not even sure how to answer this, except to say that while I am not trying to turn "PHP into a version of Java", the intent of the AppError class is exactly as you state - to tie the development to a specific class [that denotes a trapable error].

    When I originally wrote the class, I was extending it, somewhat like you showed in your post.

    PHP Code:
    class FooBar extends AppError{} 
    But in the end I realized that I wasn't making use of the fact that I had a DataBaseError vs a FileReaderError. In the end, the test came down to, are these of the AppError object in some way? So, I removed the sub-classing. It still exists in a few spots for BC reasons, but will be factored out at some point.

    Quote Originally Posted by lastcraft
    That was why I added the Success wrapper as well - to restore polymorphism.
    That is an interesting take, and I am a little embarrassed I never took that step when I first started sub-classing my errors so long ago.

    Quote Originally Posted by lastcraft
    The hard coding of the class name has big and painful implicatons for other people extending your framework and for testing. Also it makes the is_error() function mysterious. Someone trying to figure out your system basically has to read the code, as it's invisible to the interfaces.
    To me there is no big or painful implication. If someone at work wants to leverage this code, then they can simply look at the API and find out what the deal is with AppError. Just like they would/should do with any function method they don't understand, whether it is native PHP or a user-created one.

    I'm sorry Marcus, but this last point really doesn't make a lot of sense to me. I see the point you are trying to drive at, but to me it doesn't fly. If someone doesn't like the way the framework works, I am not holding a gun to their head to use it. If I don't like PEAR:DB (which I don't), then I don't use it. If I don't like Smarty (which I don't) I don't use it. If I were to write modules for the PostNuke project, should I complain because I don't like the way their API interface is set up? I seriously doubt there is *any* framework where the programmer doesn't need to either a) look at the source code, or b) read an API. And that is all that is entailed by any of the new co-op students that comes through our office every 6 months to do a work term.


    Cheers,
    Keith.

  18. #18
    ********* Victim lastcraft's Avatar
    Join Date
    Apr 2003
    Location
    London
    Posts
    2,423
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    Hi...

    Sorry, I wasn't having a go at you. I think a PEAR moan got in there some how.

    Quote Originally Posted by Taoism
    When I originally wrote the class, I was extending it, somewhat like you showed in your post.
    This is fine in the context of an application, where you are already at the top of the pile and control the lower level libraries. It doesn't work well in the context of a library or framework, because the error object may have it's own descendency already. Even if it is given the same interface it would still have to be wrapped.

    You can mitigate things somewhat with...
    PHP Code:
    class AppError {
        ...
        function 
    isError($response) {
            if (! 
    method_exists($response'isError')) {
                return 
    false;
            }
            return 
    $response->isError();
        }

    The other nuisance is that AppError::isError() static calls will litter the place. A change of framework will result in changing all of these in areas unrelated to the domain of the original framework.

    Quote Originally Posted by Taoism
    To me there is no big or painful implication.
    Ok, I exaggerated a bit . I would like to follow the argument through to it's conclusion if I can...

    Quote Originally Posted by Taoism
    ...they can simply look at the API and find out what the deal is with AppError.
    The point is that the there is some counter intuitive extra information - that the error must be descended from AppError and that a callback is necessary. To understand what is happening I have to read the API for the actual error subclass, the API for the AppError parent so as to see the comment that all errors must be descended from it. I also have to read is_error() to realise it is a static callback (my IDE won't tell me). If I skim read I'll likely call is_error() directly and will never trap any errors - not a nice bug.

    All this to check an error .

    Quote Originally Posted by Taoism
    If someone doesn't like the way the framework works, I am not holding a gun to their head to use it.
    Of course not . There are "frameworks" touted around that should know better.

    Quote Originally Posted by Taoism
    If I were to write modules for the PostNuke project, should I complain because I don't like the way their API interface is set up?
    Yes. If the developer is in the same office, then whining constantly can be very effective.

    (Before I get flamed, yes I am joking).

    Quote Originally Posted by Taoism
    I seriously doubt there is *any* framework where the programmer doesn't need to either a) look at the source code, or b) read an API.
    Well no, but you want to keep down the cognitive load. Project specific idioms don't help newcomers, students or otherwise.

    Quote Originally Posted by Taoism
    Cheers,
    Keith.
    Apologies for coming on a bit strong, BTW.

    yours, Marcus
    Marcus Baker
    Testing: SimpleTest, Cgreen, Fakemail
    Other: Phemto dependency injector
    Books: PHP in Action, 97 things

  19. #19
    SitePoint Addict been's Avatar
    Join Date
    May 2002
    Location
    Gent, Belgium
    Posts
    284
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    I think there might be a significant difference between the methods proposed here by lastcraft and Taoism:
    PHP Code:
    if (AppError::is_error($result doSomeUsefulStuff($withSomeParameters)) {
         
    // handle error
     

    Maybe I'm missing something (please say so if I do), but as much as you can ask: "But what if false is a valid return value?", you can ask: "But what if an AppError is a valid return value?"... Doesn't an analogous problem exist?

    Whereas (With the Success and Failure base classes)
    PHP Code:
    $result doSomeUsefulStuff($withSomeParameters));
     if (
    $result->isError()) {
         
    // handle error
     

    doesn't seem to have this problem: if some sort of error object, or false, or true or whatever, would be a valid return value, $result->isError() would still return false. It would seem to me that the latter method would naturally result in more consistent and more readable code?

    With AppError: If an apperror would be a valid return type, it would require a rewrite of the AppError::is_error() code: the return of true or false in the terniary operator statement would have to be the other way around.

    Just some thoughts after an initial (and interesting btw) read.
    Also, to be clear, it's not my intention to criticize or glorify other people's code, I'm trying to make an objective observation here
    Per
    Everything
    works on a PowerPoint slide

  20. #20
    Non-Member coo_t2's Avatar
    Join Date
    Feb 2003
    Location
    Dog Street
    Posts
    1,819
    Mentioned
    1 Post(s)
    Tagged
    1 Thread(s)
    Quote Originally Posted by been


    With AppError: If an apperror would be a valid return type, it would require a rewrite of the AppError::is_error() code: the return of true or false in the terniary operator statement would have to be the other way around.
    Why would an AppError ever be a valid return type?
    An error should always be treated as an error, shouldn't it?

    --ed

  21. #21
    SitePoint Addict
    Join Date
    Aug 2002
    Location
    Ottawa, Ontario, Canada
    Posts
    214
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by lastcraft
    The point is that the there is some counter intuitive extra information - that the error must be descended from AppError and that a callback is necessary. To understand what is happening I have to read the API for the actual error subclass, the API for the AppError parent so as to see the comment that all errors must be descended from it. I also have to read is_error() to realise it is a static callback (my IDE won't tell me).
    Just to clarify, the AppError object is no longer subclassed, since there is no need to do this. If an error is to be returned, then an instance of AppError is returned. This simplifies where the programmer needs to look in the API down to one class.

    Quote Originally Posted by lastcraft
    If I skim read I'll likely call is_error() directly and will never trap any errors - not a nice bug.
    I wouldn't call it a bug. I would call it a lack of focus for not reading properly

    Quote Originally Posted by lastcraft
    Apologies for coming on a bit strong, BTW.

    yours, Marcus

    No need to apologise, I greatly enjoy your insights, and quite often learn a new perspective from you. I hope you never took any of my rebuttals as too harsh either. Differences of opinion are what help to create progress

    Cheers,
    Keith.

  22. #22
    SitePoint Addict
    Join Date
    Aug 2002
    Location
    Ottawa, Ontario, Canada
    Posts
    214
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by been
    Maybe I'm missing something (please say so if I do), but as much as you can ask: "But what if false is a valid return value?", you can ask: "But what if an AppError is a valid return value?"... Doesn't an analogous problem exist?
    Quote Originally Posted by been
    With AppError: If an apperror would be a valid return type, it would require a rewrite of the AppError::is_error() code: the return of true or false in the terniary operator statement would have to be the other way around.
    You are correct. If AppError could ever be a valid return type, then yes, things could fall down. But, simply, it isn't. In our code, an AppError is always (no exceptions) an error. So your "what if" wouldn't be possible

    Cheers,
    Keith.

  23. #23
    ********* Victim lastcraft's Avatar
    Join Date
    Apr 2003
    Location
    London
    Posts
    2,423
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    Hi...

    Quote Originally Posted by Taoism
    Just to clarify, the AppError object is no longer subclassed, since there is no need to do this.
    Ok, the penny has dropped . You are using a new type as an alternative to true/false/null.

    To sum up some alternative approaches...

    1) Return an error that does not look like valid data. I'll call these "out of band" errors and this is the normal solution. The AppError type is a way of creating an out of band tunnel at the expense of some loss of portability.
    2) Return a true/false response and use a separate method to get the result. The data becomes extra state for the object which will complicate it greatly. I'll call it "sticky data". This is typical of iterators as they already have data state anyway.
    3) Use a separate isError() method to see if an error has occoured. I'll call this the "sticky error" method as the error will only be reset on the next real method. This works well for object construction with "new" where no return value is available and a failure results in the object being discarded anyway.
    4) Use trigger_error() with an error handler. Something must be done immediately with the error, and so it will probably end up on some kind of Singleton queue. Actually quite portable if you don't inflict your choice of queue object on the client.
    5) Create a response wrapper (Success/Failure). Overkill in most situations.
    6) Throw an exception (PHP5 only and this is no panacea).
    7) Use die(). Rarely acceptable of course, but simple at least.
    8) Set a global (or framework wide) error variable. Yuk!

    Have I missed any?

    My own take on this is to use the appropriate method for each situation. Mix and match is not so much of a problem as the extra code clutter if you select the wrong approach.

    Quote Originally Posted by Taoism
    No need to apologise, I greatly enjoy your insights, and quite often learn a new perspective from you. I hope you never took any of my rebuttals as too harsh either. Differences of opinion are what help to create progress
    Well cheers . Actually I prefer rebuttals. It's the follow on postings that are usually the most interesting.

    yours, Marcus
    Marcus Baker
    Testing: SimpleTest, Cgreen, Fakemail
    Other: Phemto dependency injector
    Books: PHP in Action, 97 things


Bookmarks

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •