This is necessary



  • Found this little gem of a method today:


    [code]
    public bool ShowOptional()

    {

    bool bolReturn = false;



    return bolReturn;

    }



    [/code]

    This is in production code. This is not a drill.


  • Considered Harmful

    Maybe it's implementing an interface? It's not virtual so it can't be something for a derived class to override.

    Yeah, I got nothin'.



  •  Splitting assignment and returning the value into separate lines will make the code easier to debug.


  • Trolleybus Mechanic

     Oh, I see it now. That should be blnReturn.



  • @joe.edwards said:

    Maybe it's implementing an interface? It's not virtual so it can't be something for a derived class to override.

    No, not interface implementation. Just a method, free-standing on its own.



  • @Lorne Kates said:

     Oh, I see it now. That should be blnReturn.


    Over here we'd use any of bReturn, boolReturn, isReturn, bIsReturn, boolIsReturn, or isBool. Never blnReturn.

    And yes, when we use the is* naming convention for booleans, the * bit is almost never an adjective that makes sense. It's just an "advanced" form of Hungarian notation.



  • @Lorne Kates said:

     Oh, I see it now. That should be blnReturn.


    No, in this code base all bools are pre-pended with bol. And all strings get str, all ints get int and anything non-primitive gets obj.



  • @mikeTheLiar said:

    @Lorne Kates said:

     Oh, I see it now. That should be blnReturn.


    No, in this code base all bools are pre-pended with bol. And all strings get str, all ints get int and anything non-primitive gets obj.

    FUCK I hate when people do that.



  • @mikeTheLiar said:

    @Lorne Kates said:

     Oh, I see it now. That should be blnReturn.


    No, in this code base all bools are pre-pended with bol. And all strings get str, all ints get int and anything non-primitive gets obj.

    So do strings get strobj or objstr?



  • I can beat that!

    
    public function getCensoredThing() {
        $list = CensoredClass::GetCensoredThing();
        $item = $list['item'];
        //return $list['item'];
    }
    

    My favorite thing about this method is the layered WTFery. Not only does it not return anything despite having the word "get" in its name, but even if you uncommented the return statement, the second line would still be a completely pointless assignment operation.

    Best of all, because this is a PHP codebase with typically liberal use of __call and variable method names, it's difficult to be certain whether or not tihs method is actually ever called. And because of the sheer number of side-effects resulting from that static call to GetCensoredThing, it's difficult to be certain the potential effect of removing any calls to this method.

    I just looked into that code for the purpose of writing this post, and was amused to discover that somewhere down the chain of static calls, it tries to open a database connection and throws an exception if it fails. Then it forgets about the database connection and loads the data it needs via HTTP. Then it forgets about *that* data and returns null!



  • @Lorne Kates said:

    Oh, I see it now. That should be blnReturn.
    Why not booReturn for consistency (like, in line with integer, string, object etc.)?

     



  • @GNU Pepper said:

    I can beat that!
     

    I get the feeling the author had the wrong idea about variable and function scoping. Is $item declared outside the function's scope?



  • @dhromed said:

    @GNU Pepper said:

    I can beat that!
     

    I get the feeling the author had the wrong idea about variable and function scoping. Is $item declared outside the function's scope?

    In php you need the global keyword to access global variables from within a function. Unless that is what you meant? Author didn't understand php scoping as different to some other languages?


  • Trolleybus Mechanic

    @Anonymouse said:

    Why not booReturn

    Isn't that the box in the centre of the PacMan maze where the ghosts go to return to "life"?

     



  • @immibis said:

    @mikeTheLiar said:
    @Lorne Kates said:

     Oh, I see it now. That should be blnReturn.


    No, in this code base all bools are pre-pended with bol. And all strings get str, all ints get int and anything non-primitive gets obj.

    So do strings get strobj or objstr?

    Depends. Here's a selection of variables from one page:

    [code]
    RadioButtonList objRadioButtonList
    string strSqlStatement
    SqlConnection objConnection
    SqlCommand objCommand
    ListItem objListItem
    RadioButtonList objRblBreakout
    int[ ] arrIntId 
    HyperLink objHyperLink
    SmtpClient objSmtpClient
    DropDownList objDropDown
    RequiredFieldValidator objRequiredFieldValidator
    [/code]
    

    It just keeps going....



  • @Smitty said:

    FUCK I hate when people do that.

    You and me both. I've been working here for six months and a large part of what I've done in that time is refactor variable names because it drive me fucking insane.



  • As long as we're packing already known info into variable names, I propose the following:

    int intAmount;    // insufficient -- could we get more info in there about the name itself?

    int swA_ewt_intAmount;     // better -- starts-with-A, ends-with-t -- but still, how can we be sure this is a variable at all?

    int var_swA_ewt_intAmount; // almost perfect -- but let's be realistic, sometimes we'll end up with more than one naming convention in a project!

    int extended_hungarian_with_startswith_endswith_type_and_var_indicator_var_swA_ewt_intAmount; // perfect!

     



  •  People who don't understand how to name variables...

     Wrong example:

    double InchesToPixels(double dblInches)

    {

     double  dblPixels = dblInches * 72;

     return dblPixels;

    }

     

    Better example:

     

    double InchesToPixels(double inchInput)

    {

     double  pxReturn = inchInput * (apitogettherightratioforthedevice());

     return pxReturn;

    }



  • That reminds me of the totally pointless code I saw today in our production codebase:

    liLeft = Microsoft.VisualBasic.PixelsToTwipsX(Me.Left) / Microsoft.VisualBasic.TwipsPerPixelX

  • Discourse touched me in a no-no place

    @bgodot said:

     People who don't understand how to name variables...

     Wrong example:

    double InchesToPixels(double dblInches)

    {

     double  dblPixels = dblInches * 72;

     return dblPixels;

    }

     

    Better example:

     

    double InchesToPixels(double inchInput)

    {

     double  pxReturn = inchInput * (apitogettherightratioforthedevice());

     return pxReturn;

    }

    So the difference between Apps hungarian and System's Hungarian then?


Log in to reply