SitePoint Sponsor

User Tag List

Page 1 of 2 12 LastLast
Results 1 to 25 of 30
  1. #1
    SitePoint Zealot mr_jeep's Avatar
    Join Date
    Feb 2004
    Location
    Canada
    Posts
    131
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    Best practice ???

    Hi, I'm wondering what's the best practice in theses 3 exemples :

    PHP Code:
    // #1

    function get($name)
    {
        if (isset(
    $this->cfg[$name])) {            
            return 
    $this->cfg[$name];
        }
        
        return 
    raiseError('Cannot retrieve ' $name ' config informations.');
    }

    // #2

    function get($name)
    {
        if (!isset(
    $this->cfg[$name])) {            
            return 
    raiseError('Cannot retrieve ' $name ' config informations.');
        }
        
        return 
    $this->cfg[$name];
    }

    // #3
     
    function get($name)
    {
        if (!isset(
    $this->cfg[$name])) {            
            return 
    raiseError('Cannot retrieve ' $name ' config informations.');
        } else {    
            return 
    $this->cfg[$name];
        }

    Thanks You

  2. #2
    SitePoint Evangelist
    Join Date
    Dec 2003
    Location
    Arizona
    Posts
    411
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Is there a way to design the code so that you do not have multiple return statements?

    JT
    Last edited by seratonin; Feb 13, 2004 at 17:54.

  3. #3
    SitePoint Zealot mr_jeep's Avatar
    Join Date
    Feb 2004
    Location
    Canada
    Posts
    131
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    well I think it would be :

    PHP Code:
    function get($name
    {
       if (isset(
    $this->cfg[$name])) {             
            
    $value $this->cfg[$name]; 
        } else {
            
    $value raiseError('Cannot retrieve ' $name ' config informations.'); 
        }

        return 
    $value;


  4. #4
    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)
    What is wrong with multiple return statements?
    Jason Sweat ZCE - jsweat_php@yahoo.com
    Book: PHP Patterns
    Good Stuff: SimpleTest PHPUnit FireFox ADOdb YUI
    Detestable (adjective): software that isn't testable.

  5. #5
    SitePoint Evangelist ghurtado's Avatar
    Join Date
    Sep 2003
    Location
    Wixom, Michigan
    Posts
    591
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    AFAIAC, there is nothing wrong at all with multiple return statements. Aditionally, I see no difference (other than style) in the 3 examples you provide, so choose whichever one fits you best and you think is easiest to read. As a matter of personal preference, I would probably use #3 "Backwards" (the error last), but I dont think it matters at all.

  6. #6
    SitePoint Zealot mr_jeep's Avatar
    Join Date
    Feb 2004
    Location
    Canada
    Posts
    131
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    well there is nothing wrong. All 3 examples will give the same results, although, I'm just wondering if one of the 3 is more "prefered"

  7. #7
    SitePoint Evangelist
    Join Date
    Dec 2003
    Location
    Arizona
    Posts
    411
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by sweatje
    What is wrong with multiple return statements?
    IMHO, multiple return statements make the code less readable. If the code was more complex, I think that it would be more difficult to understand if it exited in several different places. I try to avoid multiple return statements as much as possible. Similarily, I try to avoid GOTO statements. I guess it was jut engrained in me when I first learned how to program.

    JT

  8. #8
    SitePoint Wizard Ren's Avatar
    Join Date
    Aug 2003
    Location
    UK
    Posts
    1,060
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    PHP Code:
    // #4

    function get($name)
    {
        return isset(
    $this->cfg[$name]) ? $this->cfg[$name] : raiseError('Cannot retrieve ' $name ' config informations.');


  9. #9
    SitePoint Zealot sleepeasy's Avatar
    Join Date
    Sep 2003
    Location
    Bristol, UK
    Posts
    145
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    All 3 methods are fine, and there's nothing wrong with having multiple return statements.

    I personally use method #2.
    Always open to question or ridicule

  10. #10
    Non-Member
    Join Date
    Jan 2004
    Location
    Planet Earth
    Posts
    1,764
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Number One

  11. #11
    SitePoint Enthusiast
    Join Date
    Dec 2003
    Location
    Bishkek
    Posts
    74
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Ren, less readable code.
    Prefer #3

  12. #12
    SitePoint Guru
    Join Date
    Dec 2003
    Location
    oz
    Posts
    819
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Because there is often more code in a function and you have pre-conditions to check before doing the work in the method, I use this.

    Code:
    function get($name) 
    {
        // check pre-condition(s)
        if (some error){
            // raise an error
            // return
        } 
    
        // main body of method and its calculations here
    
        // return value(s) here; 
    }
    Extra return values make the code more readable. Otehrwise you would have nested if statements.

    Code:
    function get($name) 
    {
        // check pre-condition(s)
        if (some error){
        {
            // raise an error
        }
        else // NO RETURN STATEMENT, SO THIS 'ELSE' IS NEEDED
        {
            // main body of method and its calculations below here
    
            if (another error) 
            {
                 // set error
            }
            else // AND AGAIN
            {
    
                if (another error) 
                {
                     // set error
                }
                else // AND AGAIN
                {
                    // THIS IS INDENTED 4 TIMES, WHEREAS RETURNING AN ERROR EACH 
                    // TIME WOULD MAKE THIS BE NOT EVEN ONCE NESTED !!
                }    
            }
        }
    }

  13. #13
    SitePoint Wizard Ren's Avatar
    Join Date
    Aug 2003
    Location
    UK
    Posts
    1,060
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by puzanov
    Ren, less readable code.
    Prefer #3
    I don't think this simple case is any less readable than any other method.

  14. #14
    SitePoint Guru
    Join Date
    Nov 2002
    Posts
    841
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    PHP Code:
    // #5
    function get($name) {
        if (isset(
    $this->cfg[$name])) return $this->cfg[$name]; 

        return 
    raiseError('Cannot retrieve ' $name ' config informations.'); 

    #1 is Ok.
    #2 is good because it makes preconditions explicit as lazy_yogi explained.
    #3 is good because it avoids the negation. People get negation logic wrong. (No two people are not on fire)
    #4 is bad because it requires left and right scrolling and the ?: operator is hard to read even with whitespace.
    #5 is an accident waiting to happen. It drives me nuts whenever I see someone leave the braces off of an if statement.

  15. #15
    SitePoint Addict jtresidder's Avatar
    Join Date
    Nov 2003
    Location
    Southampton, UK
    Posts
    345
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Selkirk
    People get negation logic wrong.
    I don't never get no negative logic wrong, so there.

    *Can opened; worms everywhere.*

    Oh, and just to try and be useful: I prefer the third option the other way around myself.

  16. #16
    ********* Wizard silver trophy Cam's Avatar
    Join Date
    Aug 2002
    Location
    Burpengary, Australia
    Posts
    4,495
    Mentioned
    0 Post(s)
    Tagged
    1 Thread(s)
    Quote Originally Posted by Selkirk
    #4 is bad because it requires left and right scrolling and the ?: operator is hard to read even with whitespace.
    That's the method I'd probably use. If you're developing a public code library then perhaps conform to one of the wider standards out there but for your own development you can code however you want to.

  17. #17
    SitePoint Guru
    Join Date
    Dec 2003
    Location
    oz
    Posts
    819
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Selkirk
    PHP Code:
    // #5
    function get($name) {
        if (isset(
    $this->cfg[$name])) return $this->cfg[$name]; 

        return 
    raiseError('Cannot retrieve ' $name ' config informations.'); 

    #5 is an accident waiting to happen. It drives me nuts whenever I see someone leave the braces off of an if statement.
    This is a hard one to argue.
    I see your point, however as programs get larger in size, I'd like to see as much of the code as possible right on the screen in front of me. So I prefer the first to the second due to its consiceness. That's one of the many many reasons why I love python.

    PHP Code:
    function get($name) {
        if (! isset(
    $this->cfg[$name])) return error

        
    // rest of function 
    }
    function 
    get($name) {
        if (! isset(
    $this->cfg[$name])) 
        {
            return 
    error
        }

        
    // rest of function 


  18. #18
    SitePoint Evangelist ucahg's Avatar
    Join Date
    Apr 2001
    Location
    Sarnia, Ontario, Canada
    Posts
    434
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    I would personally use #2.

    It makes more sense that if an error occurs, you return immediately. If there is no error, then at the end of the function the right value is returned.

    Much easier to understand once your function gets long, in my opinion.
    Love it? Hate it? Helpful? Useless?
    Use the rate button to let me know what you think of my post!

  19. #19
    SitePoint Enthusiast Remy's Avatar
    Join Date
    Oct 2002
    Location
    Amsterdam
    Posts
    47
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Selkirk
    #4 is bad because it requires left and right scrolling and the ?: operator is hard to read even with whitespace.
    Than write it differently
    PHP Code:
    function get($name)
      {
          return isset(
    $this->cfg[$name])
              ? 
    $this->cfg[$name]
              : 
    raiseError('Cannot retrieve '.$name.' config informations.');
      } 
    I don't use this one all the time, but in this case I would. I only use ?:; when it is one line of code and looks readible(=you get it they first time when you look at it).

    -Rémy

  20. #20
    Non-Member
    Join Date
    Jan 2004
    Location
    Planet Earth
    Posts
    1,764
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    You will have a laugh at this I use ? a lot of the time as well, and it really annoys me that I cannot read the line w/out having to scroll.

    Now, why didn't I think of using seperate lines ?

    Will try to remember from now on though

  21. #21
    SitePoint Wizard Ren's Avatar
    Join Date
    Aug 2003
    Location
    UK
    Posts
    1,060
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Selkirk
    [PHP]
    #4 is bad because it requires left and right scrolling and the ?: operator is hard to read even with whitespace.
    left/right scrolling? Doesnt need to here. and this is all down to personal preferences screen-width/font-size/wallet.

    I wouldn't class it hard to read either, it just as hard easy to read as other single character tokens in PHP, even possibly easier than
    "this is a string with a $variable replacement", which is common place.

  22. #22
    SitePoint Addict
    Join Date
    Aug 2002
    Location
    Ottawa, Ontario, Canada
    Posts
    214
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    I use the ternary (?:) operator all the time, and it is perfectly readable to me (though you all seem to use a lot more whitespace than I do - I personally can't stand extra whitespace in code).

    So for me, Ren's example (minus the whitespace) is the best ;p
    PHP Code:
    <?php
    // #4 

    function get($name){ 
      return (isset(
    $this->cfg[$name]))?$this->cfg[$name]:raiseError("Cannot retrieve $name config informations.");
    }
    ?>
    Quote Originally Posted by sweatje
    What is wrong with multiple return statements?
    As a generalization, I have always been under the impression that mutlitple returns in a function/method is a bad thing to do. To me (again in general), multiple returns in a block are a bad code-smell.

    Note, I say as a generalization. I have in the past coded multiple returns into a method. However, most times, when I go back a refactor, I can usually re-work the method to have a single return, at the end of it without much trouble at all.

    Cheers,
    Keith.

  23. #23
    SitePoint Addict
    Join Date
    Aug 2002
    Location
    Ottawa, Ontario, Canada
    Posts
    214
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Ren
    ...even possibly easier than "this is a string with a $variable replacement", which is common place.
    I find this easy to read, since my editor (PHPEdit) is "smart" enough to highlight variables embeded in double quotes in a different way.

    But, I guess as everyone who has posted in this thread demonstrates.... perspective is everything

    Cheers,
    Keith.

  24. #24
    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 Taoism
    As a generalization, I have always been under the impression that mutlitple returns in a function/method is a bad thing to do. To me (again in general), multiple returns in a block are a bad code-smell.

    Note, I say as a generalization. I have in the past coded multiple returns into a method. However, most times, when I go back a refactor, I can usually re-work the method to have a single return, at the end of it without much trouble at all.
    I tend to weigh each situation on its own. If a function/method has 2, 3, or 4(okay, so that's probably alot ) return statements, and it's still easy to follow, then I don't see a reason to worry about it. When it starts getting hard to follow is when it should be recoded.

    --ed

  25. #25
    SitePoint Guru
    Join Date
    Nov 2002
    Posts
    841
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Taoism
    As a generalization, I have always been under the impression that mutlitple returns in a function/method is a bad thing to do. To me (again in general), multiple returns in a block are a bad code-smell.
    That idea stems from the old days of structural programming and "goto considered harmful." In a world of spaghetti goto code, coding gurus talked about routines with a single entry point and a single exit point being highly desirable.

    That argument has won. PHP doesn't even have a goto statement.

    Now, I think the dynamic is a continuum between nesting/indentation and multiple exit points. I feel that adding extra exit points can be clarifying when it helps reduce the amount of nesting in a routine.


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
  •