And how did return work again?



  • I am currently working on an Onlineshop, doing some tuning and such.

    And of course there has been someone before me, who has been doing it.

    The function is simply to return some strings like banking code, tax numbers and such.

    Well however, here's the code (the data replaced with XXX)

    function pdffooter ($item)
    {
    $steuernummer="XXX";
    $bank="XXX";
    $kto="XXX";
    $blz="XXX";
    $str="XXX";

    switch ($item)
    {

        case "steuernummer":
    $result=$steuernummer;
            break;

        case "bank":
    $result=$bank;
            break;

        case "kto":
    $result=$kto;
            break;

        case "blz":
    $result=$blz;
            break;

        case "str":
    $result=$str;
            break;

       
    }
    return $result;

    }



  • Good work, you found a way to save some characters, give yourself a pat in the back.



  •  It's a single-exit-point cultist!



  • @SiGe said:

    I am currently working on an Onlineshop, doing some tuning and such.

    And of course there has been someone before me, who has been doing it.

    The function is simply to return some strings like banking code, tax numbers and such.

    Well however, here's the code (the data replaced with XXX)

    Clearly you know how return works, because you hit it after every fucking sentence.


  •  Doing it this way makes it easier for debugging/logging etc. I guess you never heard about structured programming?



  •  @bstorer said:

    And don't give me any crap about hitting Enter and not Return.<input name="ctl00$ctl00$bcr$bcr$ctl00$PostList$ctl05$ctl23$ctl01" id="ctl00_ctl00_bcr_bcr_ctl00_PostList_ctl05_ctl23_ctl01_State" value="value:Filed%20under%3A%20%3Ca%20href%3D%22%2Ftags%2FThe%2BAtlantic%2BOcean%2Bcannot%2Bstop%2Bcurses%2Fdefault.aspx%22%20rel%3D%22tag%22%3EThe%20Atlantic%20Ocean%20cannot%20stop%20curses%3C%2Fa%3E%2C%20%3Ca%20href%3D%22%2Ftags%2FAnd%2Bdon_2700_t%2Bgive%2Bme%2Bany%2Bcrap%2Babout%2Bhitting%2BEnter%2Band%2Bnot%2BReturn_2E00_%2Fdefault.aspx%22%20rel%3D%22tag%22%3EAnd%20don't%20give%20me%20any%20crap%20about%20hitting%20Enter%20and%20not%20Return.%3C%2Fa%3E" type="hidden">

    Okay look, it says "Enter" right there on the fucking button, so that's what it bloody well is.



  • @mol1111 said:

     Doing it this way makes it easier for debugging/logging etc.
     

    Just as dubious a claim as the strict vs loose typing debate. The correct answer is: "I choose single-exit or multi-exit depending on the situation".

    <autoritay>

    If this devolves into that kind of pointless bickering, I'm locking the thread, because I really don't care for it.

    </autoritay>



  • @dhromed said:

     It's a single-exit-point cultist!

     

    Saves the IDE from whining about "possibly failing to return a value" or whatever warning that is. Since you're not validating the input in any way, I could send "faceboogers" and it'll fall right through the switch statement.

    Edit: Of course it just occurred to me that that's another error-- in that case you failed to initialize $result at all. This code just sucks.



  • @blakeyrat said:

    @dhromed said:

     It's a single-exit-point cultist!

     

    Saves the IDE from whining about "possibly failing to return a value" or whatever warning that is. Since you're not validating the input in any way, I could send "faceboogers" and it'll fall right through the switch statement.

    Edit: Of course it just occurred to me that that's another error-- in that case you failed to initialize $result at all. This code just sucks.

    That really is a WTF.  I expect every PDF I create to have "FACEBOOGERS" in the footer.


  • HELLo!



    It's not only the use of $result variable which is an issue here, but the bigger WTF (in my mind) is the completely superfluous switch-case block:

    function pdffooter ($item) {
    $steuernummer="XXX";
    $bank="XXX";
    $kto="XXX";
    $blz="XXX";
    $str="XXX";

    return ${$item};
    }

    Clearly the author(s) haven't spent enough time in the PHP manual...

    PS: Hopefully the HTML will work, been hearing scary stories about CS. *cowers*

    Happy ocdin'!


  • @dhromed said:

    @mol1111 said:

     Doing it this way makes it easier for debugging/logging etc.
     

    Just as dubious a claim as the strict vs loose typing debate. The correct answer is: "I choose single-exit or multi-exit depending on the situation".

     

    And the advantage of multi-exit solution are?



  • @Hellkeepa said:

    HELLo!



    It's not only the use of $result variable which is an issue here, but the bigger WTF (in my mind) is the completely superfluous switch-case block:

    function pdffooter ($item) {
        $values = array();
        $values['steuernummer']="XXX";
        $values['bank']="XXX";
        $values['kto']="XXX";
        $values['blz']="XXX";
        $values['str']="XXX";
    
        return $values[$item];
    }

    Clearly the author(s) haven't spent enough time in the PHP manual...



    PS: Hopefully the HTML will work, been hearing scary stories about CS. cowers



    Happy ocdin'!


    FTFY. It no longer has the ability to grab globals, and avoids unnecessary use of variable-variables. I was originally going to make it check to see if the value existed in the array as well and make it return NULL if it didn't, but that was pointless as it'd return NULL anyways.



  • @mol1111 said:

    @dhromed said:

    @mol1111 said:

     Doing it this way makes it easier for debugging/logging etc.
     

    Just as dubious a claim as the strict vs loose typing debate. The correct answer is: "I choose single-exit or multi-exit depending on the situation".

     

    And the advantage of multi-exit solution are?

    Less, cleaner code.  If you're such a goddamn retard that you can't figure out how to do debugging without a single exit point, then you have no fucking business writing code in the first place.



  • @Lingerance said:

    @Hellkeepa said:
    HELLo!

    It's not only the use of $result variable which is an issue here, but the bigger WTF (in my mind) is the completely superfluous switch-case block:

    function pdffooter ($item) {
        $values = array(
          'steuernummer'  => 'XXX',
          'bank'          => 'XXX',
          'kto'           => 'XXX',
          'blz'           => 'XXX',
          'str'           => 'XXX',
        ); 
    
    return $values[$item];
    

    }



    Clearly the author(s) haven't spent enough time in the PHP manual...



    PS: Hopefully the HTML will work, been hearing scary stories about CS. cowers



    Happy ocdin'!


    FTFY. It no longer has the ability to grab globals, and avoids unnecessary use of variable-variables. I was originally going to make it check to see if the value existed in the array as well and make it return NULL if it didn't, but that was pointless as it'd return NULL anyways.

    FTFY.  It no longer has overly-verbose declaration of the associative array.  Also, converted the XXX literals to use single-quoted strings because there's no need for interpolation.



  • @morbiuswilters said:

    If you're such a goddamn retard that you can't figure out how to do debugging without a single exit point, then you have no fucking business writing code in the first place.

     

    Oh, hello morbiuswilters. Pleasant and informative reply as always. Have a nice day!



  • @mol1111 said:

    @morbiuswilters said:

    If you're such a goddamn retard that you can't figure out how to do debugging without a single exit point, then you have no fucking business writing code in the first place.

     

    Oh, hello morbiuswilters. Pleasant and informative reply as always. Have a nice day!

    You were warned not to ask stupid questions.



  • @morbiuswilters said:

    @Lingerance said:

    @Hellkeepa said:
    HELLo!



    It's not only the use of $result variable which is an issue here, but the bigger WTF (in my mind) is the completely superfluous switch-case block:

    function we2smbiz ($item) {
        $values = array(
          'steuernummer'  => 'XXX',
          'bank'          => 'XXX',
          'kto'           => 'XXX',
          'blz'           => 'XXX',
          'str'           => 'XXX',
        ); 
    
    return isset($values[$item]) ? $values[$item] : null;
    

    }



    Clearly the author(s) haven't spent enough time in the PHP manual...



    PS: Hopefully the HTML will work, been hearing scary stories about CS. cowers



    Happy ocdin'!


    FTFY. It no longer has the ability to grab globals, and avoids unnecessary use of variable-variables. I was originally going to make it check to see if the value existed in the array as well and make it return NULL if it didn't, but that was pointless as it'd return NULL anyways.

    FTFY.  It no longer has overly-verbose declaration of the associative array.  Also, converted the XXX literals to use single-quoted strings because there's no need for interpolation.

    FTFY. It no longer throws a notice if called with an unexpected argument, and the function name is now completely random instead of just mostly.



  • @derula said:

    @morbiuswilters said:

    @Lingerance said:

    @Hellkeepa said:
    HELLo!



    It's not only the use of $result variable which is an issue here, but the bigger WTF (in my mind) is the completely superfluous switch-case block:

    function we2smbiz ($item) {
        $values = array(
          'steuernummer'  => 'XXX',
          'bank'          => 'XXX',
          'kto'           => 'XXX',
          'blz'           => 'XXX',
          'str'           => 'XXX',
        ); 
    
    return array_key_exists($item, $values) ? $values[$item] : null;
    

    }



    Clearly the author(s) haven't spent enough time in the PHP manual...



    PS: Hopefully the HTML will work, been hearing scary stories about CS. cowers



    Happy ocdin'!


    FTFY. It no longer has the ability to grab globals, and avoids unnecessary use of variable-variables. I was originally going to make it check to see if the value existed in the array as well and make it return NULL if it didn't, but that was pointless as it'd return NULL anyways.

    FTFY.  It no longer has overly-verbose declaration of the associative array.  Also, converted the XXX literals to use single-quoted strings because there's no need for interpolation.

    FTFY. It no longer throws a notice if called with an unexpected argument, and the function name is now completely random instead of just mostly.

    FTFY. It no longer throws a notice if called with an unexpected argument, and the function name is now completely random instead of just mostly.



  • @derula said:

    @morbiuswilters said:

    @Lingerance said:

    @Hellkeepa said:
    HELLo!

    It's not only the use of $result variable which is an issue here, but the bigger WTF (in my mind) is the completely superfluous switch-case block:

    function we2smbiz ($item) {
        $values = array(
          'steuernummer'  => 'XXX',
          'bank'          => 'XXX',
          'kto'           => 'XXX',
          'blz'           => 'XXX',
          'str'           => 'XXX',
        ); 
    
    return isset($values[$item]) ? $values[$item] : null;
    

    }



    Clearly the author(s) haven't spent enough time in the PHP manual...



    PS: Hopefully the HTML will work, been hearing scary stories about CS. cowers



    Happy ocdin'!


    FTFY. It no longer has the ability to grab globals, and avoids unnecessary use of variable-variables. I was originally going to make it check to see if the value existed in the array as well and make it return NULL if it didn't, but that was pointless as it'd return NULL anyways.

    FTFY.  It no longer has overly-verbose declaration of the associative array.  Also, converted the XXX literals to use single-quoted strings because there's no need for interpolation.

    FTFY. It no longer throws a notice if called with an unexpected argument, and the function name is now completely random instead of just mostly.

    I assumed you weren't stupid and set error reporting to ignore E_NOTICE.  I should have known better.



  • Okay, so no pintless bickering on exit points, but instead we gangrape the OP's function.

    I'm in two minds right now.

    Goddamn you all.



  • @Lingerance said:

    @derula said:
    @morbiuswilters said:

    @Lingerance said:

    @Hellkeepa said:
    HELLo!

    It's not only the use of $result variable which is an issue here, but the bigger WTF (in my mind) is the completely superfluous switch-case block:

    function we2smbiz ($wx6cymka) {
        $wp1aadlv= array(
          'steuernummer'  => 'XXX',
          'bank'          => 'XXX',
          'kto'           => 'XXX',
          'blz'           => 'XXX',
          'str'           => 'XXX',
        ); 
    
    return array_key_exists($wx6cymka, $wp1aadlv) ? $wp1aadlv[$wx6cymka] : null;
    

    }


    Clearly the author(s) haven't spent enough time in the PHP manual...

    PS: Hopefully the HTML will work, been hearing scary stories about CS. *cowers*

    Happy ocdin'!

    FTFY. It no longer has the ability to grab globals, and avoids unnecessary use of variable-variables. I was originally going to make it check to see if the value existed in the array as well and make it return NULL if it didn't, but that was pointless as it'd return NULL anyways.

    FTFY.  It no longer has overly-verbose declaration of the associative array.  Also, converted the XXX literals to use single-quoted strings because there's no need for interpolation.

    FTFY. It no longer throws a notice if called with an unexpected argument, and the function name is now completely random instead of just mostly.
    FTFY. It no longer throws a notice if called with an unexpected argument, and the function name is now completely random instead of just mostly.

     FTFY. If you are obscuring the function name, why not go all the way. Your competitors are no longer able to steal your code and use it.



  • @Abdiel said:

    @Lingerance said:

    @derula said:
    @morbiuswilters said:

    @Lingerance said:

    @Hellkeepa said:
    HELLo!

    It's not only the use of $result variable which is an issue here, but the bigger WTF (in my mind) is the completely superfluous switch-case block:

    function um5pnyeo ($gl9iamze, $cy7vitma) {
    return array_key_exists($gl9iamze, $cy7vitma);
    }

    function mn3crbas ($vi9ruqun) {
    switch ($vi9ruqun)
    {

    case 'steuernummer':
    

    $result='get1owpo';
    break;

    case 'bank':
    

    $result='zou+vitp';
    break;

    case 'kto':
    

    $result='sa)sicoz';
    break;

    case 'blz':
    

    $result='ken?cyas';
    break;

    case 'str':
    

    $result='aq]erufe';
    break;

    }
    return $result;
    }

    function we2smbiz ($wx6cymka) {

    $wp1aadlv= array( 'get1owpo' => 'XXX', 'zou+vitp' => 'XXX', 'sa)sicoz' => 'XXX', 'ken?cyas' => 'XXX', 'aq]erufe' => 'XXX', ); $profit = um5pnyeo($wx6cymka, $wp1aadlv) ? $wp1aadlv[mn3crbas($wx6cymka)] : null
    return $profit; }


    Clearly the author(s) haven't spent enough time in the PHP manual...

    PS: Hopefully the HTML will work, been hearing scary stories about CS. *cowers*

    Happy ocdin'!

    FTFY. It no longer has the ability to grab globals, and avoids unnecessary use of variable-variables. I was originally going to make it check to see if the value existed in the array as well and make it return NULL if it didn't, but that was pointless as it'd return NULL anyways.

    FTFY.  It no longer has overly-verbose declaration of the associative array.  Also, converted the XXX literals to use single-quoted strings because there's no need for interpolation.

    FTFY. It no longer throws a notice if called with an unexpected argument, and the function name is now completely random instead of just mostly.
    FTFY. It no longer throws a notice if called with an unexpected argument, and the function name is now completely random instead of just mostly.

     FTFY. If you are obscuring the function name, why not go all the way. Your competitors are no longer able to steal your code and use it.

    FTFY.


  • /win thread.



  • Here's how you do it

    function pdffooter ($item) { $values = array( 'steuernummer' => 'XXX', 'bank' => 'XXX', 'kto' => 'XXX', 'blz' => 'XXX', 'str' => 'XXX', ); $randomNumber = rand(0, count($values)); while ($values[$randomNumber]!=$item) { $randomNumber = rand(0, count($values)); } return $values[$randomNumber]; }


  • @Abdiel said:

    @Lingerance said:

    @derula said:
    @morbiuswilters said:

    @Lingerance said:

    @Hellkeepa said:
    HELLo!

    It's not only the use of $result variable which is an issue here, but the bigger WTF (in my mind) is the completely superfluous switch-case block:

    function we2smbiz ($wx6cymka) {
        return 'XXX';
    }


    Clearly the author(s) haven't spent enough time in the PHP manual...

    PS: Hopefully the HTML will work, been hearing scary stories about CS. *cowers*

    Happy ocdin'!

    FTFY. It no longer has the ability to grab globals, and avoids unnecessary use of variable-variables. I was originally going to make it check to see if the value existed in the array as well and make it return NULL if it didn't, but that was pointless as it'd return NULL anyways.

    FTFY.  It no longer has overly-verbose declaration of the associative array.  Also, converted the XXX literals to use single-quoted strings because there's no need for interpolation.

    FTFY. It no longer throws a notice if called with an unexpected argument, and the function name is now completely random instead of just mostly.
    FTFY. It no longer throws a notice if called with an unexpected argument, and the function name is now completely random instead of just mostly.

     FTFY. If you are obscuring the function name, why not go all the way. Your competitors are no longer able to steal your code and use it.

     

    FTFY.  



  • @derula said:

    I assumed you weren't stupid and set error reporting to ignore E_NOTICE.  I should have known better.

    TRWTF is disabling ANY error reporting during development.  ALWAYS code with E_ALL | E_STRICT.  I can see turning off error reporting on a production app, but at least log it at that point (So you can tell if there is a bug that testing didn't find).  Notices exist for a reason, and blindly disabling them on a development platform is stupid at best...  Sure, this particular example probrably won't result in dangerous code, but many other instances it can (for example, what if you did this in the global scope?  Then you'd open yourself up to a nice attack if register_globals was on).  A notice IS AN ERROR, plain and simple...



  • @ircmaxell said:

    @derula said:

    I assumed you weren't stupid and set error reporting to ignore E_NOTICE.  I should have known better.

    TRWTF is disabling ANY error reporting during development.  ALWAYS code with E_ALL | E_STRICT.  I can see turning off error reporting on a production app, but at least log it at that point (So you can tell if there is a bug that testing didn't find).  Notices exist for a reason, and blindly disabling them on a development platform is stupid at best...  Sure, this particular example probrably won't result in dangerous code, but many other instances it can (for example, what if you did this in the global scope?  Then you'd open yourself up to a nice attack if register_globals was on).  A notice IS AN ERROR, plain and simple...

    No, notices are largely useless.  If you're using register_globals, that is TRWTF.  Relying on notices to tip you off because you are using dangerous code is moronic.  The notices just whine about minor bullshit.  Wrapping every array access in is_set() is pointless code bloat.



  • @ircmaxell said:

    @derula said:
    I assumed you weren't stupid and set error reporting to ignore E_NOTICE. I should have known better.

    snip

    You sir fail at quoting.



  • @morbiuswilters said:

    Wrapping every array access in is_set() is pointless code bloat.

    True... but, on the relatively* safe assumption that array indexing isn't even going to die with a fatal error, prefixing the array variable with @ will do the trick.
    And most some NOTICEs are useful.

    * We're talking PHP here, after all...



  • @bannedfromcoding said:

    True... but, on the relatively* safe assumption that array indexing isn't even going to die with a fatal error, prefixing the array variable with @ will do the trick.

    True, but it's still extra code.  And unnecessary.

     

    @bannedfromcoding said:

    And most some NOTICEs are useful.

    Examples?  I have never found a notice I give a crap about.



  • @morbiuswilters said:

    @bannedfromcoding said:

    And most some NOTICEs are useful.

    Examples?  I have never found a notice I give a crap about.

    When developing code for something that can potentially be run on a shared host (most FOSS PHP projects have this requirement) having notices on informs you of potential problems, they've also helped me debug issues where the array index I was using wasn't always being set. Largely they're annoying yes, but I find them useful. We actually have them enabled on prod, sent to syslog.



    As for the specific case of the missing index in an array I have a wrapper function for that (from memory):

    function getArrayValue($key, $array, $default = NULL) {
        return (array_key_exists($key, $array)) ? $array[$key] : $default;
    }
    

    Mostly used on $_GET/$_POST, most other array sources I don't check explicitly as they're checked implicitly somewhere else (like if when getting a row from the DB, if the query failed I can assume the array is shit, otherwise it's good).



  • @Lingerance said:

    When developing code for something that can potentially be run on a shared host (most FOSS PHP projects have this requirement) having notices on informs you of potential problems...

    I'm not sure what shared hosting has to do with it.  Still, I've never found notices to be useful.

     

    @Lingerance said:

    As for the specific case of the missing index in an array I have a wrapper function for that (from memory):

    Meh, it's more verbose, adds function overhead and gets ugly real quick with multi-dimensional arrays.



  • @morbiuswilters said:

    Still, I've never found notices to be useful.

    From our production log ("why are our orders not coming through?!"):

    [Tue Mar 23 08:19:50 2010] (Language.class.php:25) [PHP notice] Undefined index:  languageext
    [Tue Mar 23 08:19:50 2010] (Language.class.php:25) [PHP notice] Undefined index:  country

    A lot of those errors are because of misspelt variables or forgetting to use a $, so PHP assumes you meant a constant (which was not defined) someVar instead of a variable $someVar. (The latter would result in having the actual string value "someVar".

     @morbiuswilters said:

    Meh, it's more verbose, adds function overhead and gets ugly real quick with multi-dimensional arrays.

    Which is why I use isset/empty: if (isset($array[$key1][$key2]['foo'])) // will not trigger a notice if anything is unset

    (First post; be gentle. And where is the preview function?!)



  • @morbiuswilters said:

    @Lingerance said:

    When developing code for something that can potentially be run on a shared host (most FOSS PHP projects have this requirement) having notices on informs you of potential problems...

    I'm not sure what shared hosting has to do with it.  Still, I've never found notices to be useful.

    They almost always have register_globals on, as such you actually have to code to handle the case where they're on.
    @morbiuswilters said:

    @Lingerance said:

    As for the specific case of the missing index in an array I have a wrapper function for that (from memory):

    Meh, it's more verbose, adds function overhead and gets ugly real quick with multi-dimensional arrays.

    At which point that function isn't used, it's meant to get a value or provide the default if not available, which is one of the more common times we access an array. We don't do that for every array access, as I stated in the other post, there are places where we just assume the index is set (like when we successfully pull from the DB), but for stuff we're unsure of ($_GET/$_POST) it's nice, we can have a default page to view if not present in the GET params and the like..



  • @mobiusquilters said:

    A lot of those errors are because of misspelt variables or forgetting to use a $, so PHP assumes you meant a constant (which was not defined) someVar instead of a variable $someVar. (The latter would result in having the actual string value "someVar".

    Then that's just shitty code.  And your unit tests and code reviews should catch it.

     

    @mobiusquilters said:

    Which is why I use isset/empty: if (isset($array[$key1][$key2]['foo'])) // will not trigger a notice if anything is unset

    It's still extra code and more overhead.  I understand how the isset() idiom works, I just hate it.

     

    @mobiusquilters said:

    (First post; be gentle. And where is the preview function?!)

    There's a preview tab in the editor.  I'll try to be gentle, but it's a bit hard with your username.  It's like looking into a mirror while I masturbate; I just can't control myself.



  •  @morbiuswilters said:

    True, but it's still extra code.  And unnecessary

    It's a matter of trusting user input...  Plus, using isset with the terinary syntax lets you specify a non-null default, so rather than doing:

    $foo = $_GET['bar'];

    if (is_null($foo)) {

        $foo = 'something else';

    }

    I simply do:

    $foo = isset($_GET['bar']) ? $_GET['bar'] : 'something else';

    Even in the cases where I want null, I still use the terinary syntax because I find it easier to read.  Sure, you know that an unset variable would return null, but with the isset syntax, you can quickly tell if you would expect the variable to be unset.

     

    As far as notices never being useful, I think it depends on your philosiphy.  I believe that any time I access an undefined variable, that something went wrong.  Either it's as simple as I forgot to define it, or as lazy as "it's just an array index that wasn't supplied".  Either way, I see it as an error condition.  Not all error conditions are critical, but I do believe that all are important.  Notices will quickly find mis-spelled and miss-named variables, as well as undefined constants and other things that your IDE or debugger may miss.  If you don't like notices, than that's fine for you.  But I will tell you this, you won't be working for me.  $foo = $_GET['bar']; would fail any code review that I would do.  Sure it adds a little bit of extra code to add the isset, but IMHO it makes the entire application stronger and more secure. If I try an app, and it throws notices, it tells me a lot about the programmer who wrote it (that they are typically either lazy or incompetent.  Either one I do not want)...

     And never, ever use the @ syntax in production code.  It can lead to the WSOD (White Screen Of Death) if the resulting error is greater than a warning.  If you've ever had the pleasure of debugging a WSOD, then you'll quickly understand why MOST uses of it are evil.  I'm not saying that it doesn't have its uses, but those uses are the exception as opposed to the rule...



  • @Lingerance said:

    They almost always have register_globals on, as such you actually have to code to handle the case where they're on.

    In this day-and-age?  Jesus.  Still, I'd just write some standard code that runs before everything else and unsets every global variable to get around this.

     

    @Lingerance said:

    At which point that function isn't used, it's meant to get a value or provide the default if not available, which is one of the more common times we access an array. We don't do that for every array access, as I stated in the other post, there are places where we just assume the index is set (like when we successfully pull from the DB), but for stuff we're unsure of ($_GET/$_POST) it's nice, we can have a default page to view if not present in the GET params and the like..

    Meh, I just prefer checking the value directly.  Coming up with ways to work around stupid notices is a waste of my time and it uglies up the code.



  • @ircmaxell said:

    Even in the cases where I want null, I still use the terinary syntax because I find it easier to read.  Sure, you know that an unset variable would return null, but with the isset syntax, you can quickly tell if you would expect the variable to be unset.

    It's "ternary".

     

    @ircmaxell said:

    As far as notices never being useful, I think it depends on your philosiphy.  I believe that any time I access an undefined variable, that something went wrong.  Either it's as simple as I forgot to define it, or as lazy as "it's just an array index that wasn't supplied".  Either way, I see it as an error condition.  Not all error conditions are critical, but I do believe that all are important.  Notices will quickly find mis-spelled and miss-named variables, as well as undefined constants and other things that your IDE or debugger may miss.

    Any of those should be caught in code reviews and unit tests, anyway.  If you want to make up rules like "every access to an undefined variable means something went wrong", that's your business, but I don't find it sensible.

     

    @ircmaxell said:

    If you don't like notices, than that's fine for you.  But I will tell you this, you won't be working for me.

    I wouldn't want to work for an anal-retentive cargo cult programmer.

     

    @ircmaxell said:

    $foo = $_GET['bar']; would fail any code review that I would do.

    Why?

     

    @ircmaxell said:

    Sure it adds a little bit of extra code to add the isset, but IMHO it makes the entire application stronger and more secure.

    And this is why notices are bad.  Data sanitization is far more complex than just making sure your runtime doesn't throw notices.  The problem is it leads people like you to think there is something inherently "pure" and more secure about notice-free code.  Cargo cult.

     

    @ircmaxell said:

    And never, ever use the @ syntax in production code.  It can lead to the WSOD (White Screen Of Death) if the resulting error is greater than a warning.  If you've ever had the pleasure of debugging a WSOD, then you'll quickly understand why MOST uses of it are evil.  I'm not saying that it doesn't have its uses, but those uses are the exception as opposed to the rule...

    If the code generates an error, you're going to get a WSOD no matter what.  The @ just makes it impossible to.. oh shit, you work with display_errors on.  That's what you meant by "debugging a WSOD".  Because the non-retarded amongst us always get white screens on errors, and debugging means checking the error log.  Although, I agree about using @ in most cases.  I only use it for file operations like unlink() where I don't care if it succeeded or not.



  • As far as notices go, we don't work together (and don't need to), so let's just agree to disagree.  I feel that while they are not "critical", they still should be silenced (it's good practice IMHO).  You believe that they are an annoyance that can be ignored.  To each their own...

     @morbiuswilters said:

    If the code generates an error, you're going to get a WSOD no matter what.  The @ just makes it impossible to.. oh shit, you work with display_errors on.  That's what you meant by "debugging a WSOD".  Because the non-retarded amongst us always get white screens on errors, and debugging means checking the error log.  Although, I agree about using @ in most cases.  I only use it for file operations like unlink() where I don't care if it succeeded or not.

    I always develop with display_errors on.  And I always turn it off on production systems.  And I normally use error exceptions, so even though the error may not display, the code will treat the error as an exception and exit gracefully (logging a full backtrace to logs).  The combination lets me develop quite efficiently (if I encounter an error, I don't need to go digging through logs to find it.  But it's still there if I need it).  Overkill?  Sure...  But at least I know that my code will always fail gracefully (even when it didn't need to) rather than possibly go past an error that could be serious (if a Warning was thrown that wasn't checked for).  And I do use the @ command when doing unlinks from temp files or other operations that I don't care if they succeed... But I don't use them on areas where a lot of other people do.  For example, fsockopen will throw a warning if it can't connect.  Rather than prefixing with @, I use error exceptions, and wrap the connect in a try/catch block...  Sure, it's a slight bit more code, but I think it makes the whole process a lot more robust.  

     Then again, that's just my opinion and how I write code.  There's more than one way to skin a cat, and most people will usually think that their method is best...  To each their own.  The only time it matters, is if we're working on the same project (or with each others code).  But we're not, so in the end what does it really matter?



  • @ircmaxell said:

    The only time it matters, is if we're working on the same project (or with each others code).  But we're not, so in the end what does it really matter?
    You're never going to develop a mighty e-peen with an attitude like that.



  • Actually, using @ prevents inserting an error into the log, that's true.
    But I suggested using it in the [code] $var = @$_GET[]'whateverblah'];[/code] case, to prevent the NOTICE when you actually handle undefined/NULL below. Especially on a codebase like the one I work on, where the original author was using error_reporting(0).
    And if array indexing throws a fatal error... then you have a lot else to worry about.



  • @bannedfromcoding said:

    Actually, using @ prevents inserting an error into the log, that's true.
    Actually, it prevents all automated handling of the error (so if you use a custom error handler, it won't see it either).  Sure, it does give you the $php_error variable, but all your usual error handling will be bypassed (which can be aventagous depending on what you're doing).  

     @bannedfromcoding said:

    And if array indexing throws a fatal error... then you have a lot else to worry about.

    Sure.  But I still consider using @ bad practice unless it's really needed.  And I don't consider it needed when doing $var = @$_GET['blah'].  Now, that's not to say that I think what you're doing is wrong (In the realities of working on other people's code, sometimes it's just easier to toss in a few @'s than fix the underlying issue), but I do think it should be avoided in new code.  If you're in the habit of not using it to surpress error messages, then you will be far less tempted to use it in a case where you wind up surpressing a message you really don't want to surpress...  In one sense it's like goto in that (In my experience) it's abused FAR more often than used in proper situations (but unlike goto, it does have a fair number of valid use cases)...

    Again, just my $0.02.  Agree or disagree, it really doesn't matter...



  • @bstorer said:

    @ircmaxell said:

    The only time it matters, is if we're working on the same project (or with each others code).  But we're not, so in the end what does it really matter?
    You're never going to develop a mighty e-peen with an attitude like that.

    Yeah, did I wander onto "The Daily Gentleman's Disagreement Over Tea" again?



  • @ircmaxell said:

    As far as notices go, we don't work together (and don't need to), so let's just agree to disagree.  I feel that while they are not "critical", they still should be silenced (it's good practice IMHO).  You believe that they are an annoyance that can be ignored.  To each their own...

    Holy shit, you get it.  You're one of the few people on this site who disagrees over something and doesn't feel compelled to spend hours quoting Wikipedia to try and prove their point.  Regardless, your position is basically the same as mine: to each his own.

     

    @ircmaxell said:

    The combination lets me develop quite efficiently (if I encounter an error, I don't need to go digging through logs to find it.  But it's still there if I need it).

    I only use logging.  Once again, I suppose it's a personal choice, but I don't like displaying errors ever.  Also, I'm always tailing the log file anyway, so it's not like it's hard for me to find the error.  I always try to catch fatal errors and display an end-user-friendly page.  I don't ever want a WSOD in production.

     

    @ircmaxell said:

    Overkill?

    Logging errors is never overkill.  Using display_errors because you don't want to look at a log file.. well, that strikes me as kind of lazy.  Isn't that what you just criticized me over?  :-)

     

    @ircmaxell said:

    But I don't use them on areas where a lot of other people do.

    I would never consider anything "a lot of people do" in PHP to be best practice.  I love the language, but the developer community is brain-dead.

     

    @ircmaxell said:

    For example, fsockopen will throw a warning if it can't connect.  Rather than prefixing with @, I use error exceptions, and wrap the connect in a try/catch block...  Sure, it's a slight bit more code, but I think it makes the whole process a lot more robust.

    Well, yeah.  You seem to assume I use no error checking, when in fact I use a lot of it.  Of course, I try to encapsulate as much of that in re-usable code as possible; I never call socket_create directly, I use my own libraries which have error-checking built-in.  When it comes to notices, though, I just don't care.  If I'm looking at a $_GET var and it's not set, in 99% of cases I don't care.  In the 1% that I do, I will use isset() explicitly, but making me wrap every array access in isset() is awful.

     

    Part of what bothers me about E_NOTICE is the same thing that bothers me about people who are strongly pro-strictly-typed languages.  Software engineering is hard.  The idea behind E_NOTICE seems to be that it will help people catch errors, but all it does is get people in the habit of wrapping every array access in isset().  It will also catch typo'd variables, but that's something you should be catching without the help of the runtime.  The core belief here is that we can leverage technology to catch errors and make better software.  In some cases that may hold, but the focus is entirely wrong, in my opinion.  Good software is a result of good, experienced developers and good processes.  This obsession with "syntactic strictness will save us" is fatally flawed.  No automatic solution can catch all bugs; in fact, it can't even catch the important bugs.  But finding good developers is hard, forging and refining a good process is hard.  Coming up with strict syntactic rules that can be checked automatically is easy; it's a technological band-aid for a much deeper problem.  And it creates a false sense of security; having code that runs without notices is virtually meaningless when it comes down to determining if the code does what it is supposed to, if it is maintainable and performant.  It's just re-arranging the deck chairs on the Titanic; it's just spell-checking a Ron Paul pamphlet. 

     

    And it's a cost.  All of this anal-retentiveness consumes resources, yet it does not guarantee any quality of results.  Good process, good developers; those yield quality results, consistently.  It's a much better investment of resources to focus on creating testable, maintainable code and processes that aid in quality development.  But that's hard.  So instead we get a lot of people opting for the "spend an extra 10% of your time satisfying the OCD, aspie jackoffs who wrote the language spec".  Because that's relatively easy.  It's a lot easier than picking up your co-workers code and going through line-by-line looking for logic errors.  It's a lot easier than wracking your brain to come up with test cases that break your code.  And it's easier than having frequent interaction with and feedback from your stakeholders.



  • @morbiuswilters said:

    Logging errors is never overkill.

    I was refering to the exceptions and logging backtraces.  But fair enough...

     @morbiuswilters said:

    Using display_errors because you don't want to look at a log file.. well, that strikes me as kind of lazy.  Isn't that what you just criticized me over?  :-)

    Yeah, it is I guess...

    @morbiuswilters said:

    You seem to assume I use no error checking, when in fact I use a lot of it.
    Well, that part I was talking about in general, not just to you.  But I think that it's easy enough to miss an error check that using @ can get dangerous.  Sure, it's not a critical mistake, but I feel it's bad practice.  With that said, I'm not about to petition for the @ construct to be removed from the language...

    @morbiuswilters said:

    If I'm looking at a $_GET var and it's not set, in 99% of cases I don't care.  In the 1% that I do, I will use isset() explicitly, but making me wrap every array access in isset() is awful.
    I would only make you wrap array accesses that depend on user input in an isset.  If you pull a row from a DB, if the inner array is set, you know the columns should be set.  Using isset() there is stupid.  while ($row = $result->fetch_assoc()) { $title = isset($row['title']) ? $row['title'] : null; } is just silly.  I think the whole accessing $_GET with isset vs without comes down to a personal preference like tab vs space indenting and inline if statements.  If the code guidelines that you are following say use isset($_GET['foo']) ? $_GET['foo'] : null, then use that.  If they don't, then use what you're comfortable with.  The guidelines that I set on my projects (I can set my own guidelines at my company) say to use isset if it's dealing with user input, and it's optional if not dealing with user input.  Just personal preference.

    @morbiuswilters said:

    Part of what bothers me about E_NOTICE is the same thing that bothers me about people who are strongly pro-strictly-typed languages.  Software engineering is hard.  The idea behind E_NOTICE seems to be that it will help people catch errors, but all it does is get people in the habit of wrapping every array access in isset().  It will also catch typo'd variables, but that's something you should be catching without the help of the runtime.  The core belief here is that we can leverage technology to catch errors and make better software.  In some cases that may hold, but the focus is entirely wrong, in my opinion.  Good software is a result of good, experienced developers and good processes.  This obsession with "syntactic strictness will save us" is fatally flawed.  No automatic solution can catch all bugs; in fact, it can't even catch the important bugs.  But finding good developers is hard, forging and refining a good process is hard.  Coming up with strict syntactic rules that can be checked automatically is easy; it's a technological band-aid for a much deeper problem.  And it creates a false sense of security; having code that runs without notices is virtually meaningless when it comes down to determining if the code does what it is supposed to, if it is maintainable and performant.  It's just re-arranging the deck chairs on the Titanic; it's just spell-checking a Ron Paul pamphlet.

     I must say that's one of the best writeups I have ever read on the issue...  

    @morbiuswilters said:

    And it's a cost.  All of this anal-retentiveness consumes resources, yet it does not guarantee any quality of results.

    That's the thing though, this doesn't consume any resources.  I've made it such a habbit that the code I write doesn't throw notices (And I expect that out of my developers.  Needless?  Perhaps.  But it's not something that takes a lot of brain power to get your head around.  And if you can't grasp that simple concept then I don't want you writing code for me).  And if it does, it's because of an actual bug, not because I forgot something.  It doesn't add 10% of time, because I almost never get a notice to hunt down (It's simply taken care of when I first write the code).  And how many times do you really write $foo = $_GET['foo']?  I would bet that it's an insignificant amount of your code.  So adding the extra 20 characters or so won't really make that big of an impact in typing speed...  I don't reject code from a review because it throws notices.  I reject it because there is a logic error or guideline failure somewhere in there.  I would fail $foo = $_GET['foo'] because it violates the guidelines, not because it throws the notice...  Sure, it's a fine line, but it's a line that I consider important...  

    @morbiuswilters said:

    It's a lot easier than picking up your co-workers code and going through line-by-line looking for logic errors.  It's a lot easier than wracking your brain to come up with test cases that break your code.  And it's easier than having frequent interaction with and feedback from your stakeholders.

     But I do that as well...  I don't run the software and say "well it doesn't generate any errors or notices, so it must be ok".  I run it through phpUnderControl and take a look at the code coverage (Is every single line being run by at least one test?).  I also look at the CRAP index and the N-Path complexity indexes. And only after all of those look good will I actually look at the code to see if there are any logic errors, guidelines violations (although phpUnderControl will typically catch them) or anything else I don't agree with...

    It's how I do it...  And it's seemingly worked for me up until now...



  • This conversation makes me so happy for ASP.NET.



  • @blakeyrat said:

    This conversation makes me so happy for ASP.NET.

    This conversation makes me so happy for you.



  • @ircmaxell said:

    I was refering to the exceptions and logging backtraces.  But fair enough...

    I wouldn't consider either of those excessive.  I use a custom error handler to give me full backtraces for any warnings or errors.  Of course, this is something most languages do automatically with stack traces, but PHP only generates those for uncaught exceptions.  And, really, those stack traces are rather weak compared to debug_backtrace().

     

    @ircmaxell said:

    That's the thing though, this doesn't consume any resources.  I've made it such a habbit that the code I write doesn't throw notices (And I expect that out of my developers.  Needless?  Perhaps.  But it's not something that takes a lot of brain power to get your head around.  And if you can't grasp that simple concept then I don't want you writing code for me).  And if it does, it's because of an actual bug, not because I forgot something.  It doesn't add 10% of time, because I almost never get a notice to hunt down (It's simply taken care of when I first write the code).  And how many times do you really write $foo = $_GET['foo']?  I would bet that it's an insignificant amount of your code.  So adding the extra 20 characters or so won't really make that big of an impact in typing speed...  I don't reject code from a review because it throws notices.  I reject it because there is a logic error or guideline failure somewhere in there.  I would fail $foo = $_GET['foo'] because it violates the guidelines, not because it throws the notice...  Sure, it's a fine line, but it's a line that I consider important...  

    Fair enough.  I wasn't just referring to isset(), or even to PHP.  My point was more broadly referring to tendency for people to take refuge in automated verification.  Just like the people who say "But it validates!" when their HTML doesn't work on IE.  Sorry, but IE is 80% of the market.  And so on.

     

    @ircmaxell said:

    But I do that as well...  I don't run the software and say "well it doesn't generate any errors or notices, so it must be ok".  I run it through phpUnderControl and take a look at the code coverage (Is every single line being run by at least one test?).  I also look at the CRAP index and the N-Path complexity indexes. And only after all of those look good will I actually look at the code to see if there are any logic errors, guidelines violations (although phpUnderControl will typically catch them) or anything else I don't agree with...

    It's how I do it...  And it's seemingly worked for me up until now...

    If it works for you, then good.  Having a process that works is a rarity, and this site is a testament to that.  I wasn't criticizing you and your processes, just more generally ranting about attitudes and processes I've encountered in the past.



  • @fatdog said:

    @blakeyrat said:

    This conversation makes me so happy for ASP.NET.

    This conversation makes me so happy for you.

    Aww.. The Mexican feels left out because the newest language he has access to is Turbo Pascal.



  • @morbiuswilters said:

    @fatdog said:

    @blakeyrat said:

    This conversation makes me so happy for ASP.NET.

    This conversation makes me so happy for you.

    Aww.. The Mexican feels left out because the newest language he has access to is Turbo Pascal.

    :(


Log in to reply