More New Math



  • else if( EPSILON <= (AbsoluteFloat(fValueOfInterest - 0.0F)) )

    I cannot comprehend the confusion of ideas which would provoke such a line of code (apologies to Babbage). (Hint: the WTF is not the check against EPSILON.)  In other words, there is no reason to check a difference against *anything* here, especially not against the most WTF number with which to take a difference.  (Even if the compiler hopefully optimizes that crap out...)

    And for completeness (some may say 'overkill' in lieu of a macro) - considering this is in a real-time embedded system where function calls are not free. AbsoluteFloat() is defined in a utility file and is not declared as inline:

    float AbsoluteFloat(float fInputVal)
    /*
    ** Description : Calculate absolute value of a Float
    **
    **
    **
    **
    ** Parameters
    ** Inputs : fInputVal - input value
    **
    ** Outputs : None
    **
    **
    ** Returns : Absolute value of input value
    **
    **
    **
    ** MANUAL REVISION HISTORY
    ** Date Who Description
    **
    ** *removed to protect the guilty*
    **
    **
    */
    {
    if(fInputVal < 0.0F)
    {
    /* negate input value */
    fInputVal = -fInputVal;
    }
    else
    {
    /* positive value */
    }

    return fInputVal;

    }


  • Shouldn't be necessary in C, but I've seen the "subtract 0" idiom in other languages to coerce numbers to another type.  My initial suspicion would be that fValueOfInterest isn't actually a float, or maybe that it wasn't in the past before some refactoring, and that you've got a quirky compiler for your embedded platform.



  • My initial thought is that it is simply a cut&paste from a line where two non-zero floats were being compared.



  •  fValueOfInterest?  Is this using FP for currency representation?



  • @morbiuswilters said:

     fValueOfInterest?  Is this using FP for currency representation?

     

    Nah, it's my bad attempt at anonymization (an alternative might be fSomeRandomFloatValue or fTheValueForWhichWeNeedTheAbsoluteValue - but I think that I would have conniptions for suggesting variable names of that verbosity.  Oh, wait....)

    Thankfully the machines for which I develop control systems don't have anything to do with the (mis)management of money.



  •  @Minos said:

    Shouldn't be necessary in C, but I've seen the "subtract 0" idiom in other languages to coerce numbers to another type.  My initial suspicion would be that fValueOfInterest isn't actually a float, or maybe that it wasn't in the past before some refactoring, and that you've got a quirky compiler for your embedded platform.

    Nah, fValueOfInterest here really is a float. What's really fun, though, is the constant 0.0 is a double for this compiler, but the runtime libraries don't actually implement double math, so you get linker errors. Hence the need to make it 0.0f so it doesn't do the promotion to double.

    I suspect it's more like the copy-paste idea above from Oz...this codebase is the motherlode of the copy-paste anti-pattern (there is one file where essentially one function is copied I think twenty times with only a few minor changes....trouble is it works and budget/timing constraints don't allow us to refactor it.  Luckily this is just "prototype" code that is going to be replaced in the future.)



  • @too_many_usernames said:

    Luckily this is just "prototype" code that is going to be replaced in the future.

    If you don't have time to do it right in the first place, when will you have time to come back and fix it?



  •  I honestly don't see what the WTF is. The subtraction of zero is not needed, but it's the compiler's job to optimize it out, and it makes it clearer that the intent of the code is to see if the floating point value is close enough to zero to be considered zero within the accuracy of the platform's floating point math operations. I can see the argument that AbsoluteFloat should have been inlined and is poorly written (for example, the 'else' should not be there). But it's not off-the-scale awful by any stretch of the imagination.



  • @OzPeter said:

    @too_many_usernames said:
    Luckily this is just "prototype" code that is going to be replaced in the future.
    If you don't have time to do it right in the first place, when will you have time to come back and fix it?
     

    This prototype code exists because a customer needed code running on new hardware very quickly...in an industry where you're not allowed to sell product with code that has not been through a rigorous development process. So while waiting for the rigorous process, we have "prototype" code that allows us to actually test the articles that need the controller.In effect, the prototype allows us to make sure we're getting it right the first time - something that would likely not be possible without prototyping.

    The prototype is being used to verify requirements - not actually implement those requirements.  (There is also an effect of having no fewer than three parties involved with the development of the prototype code.)



  • @too_many_usernames said:

    I suspect it's more like the copy-paste idea above from Oz...this codebase is the motherlode of the copy-paste anti-pattern (there is one file where essentially one function is copied I think twenty times with only a few minor changes....trouble is it works and budget/timing constraints don't allow us to refactor it.  Luckily this is just "prototype" code that is going to be replaced in the future evolve into a production system and haunt us for years.)

     

    FTFY.

    I read your later post, and ok, maybe your company is one of the few that understands what prototyping is.  Seems like for most of us, though, "prototype" is code for "half-assed brittle garbage that I threw together in a week without really knowing what I was doing and find myself still supporting after a year and a half."



  •  else { /* positive value */ }

     

    Instant classic

     

     



  •  What's wrong with that AbsoluteFloat function?  It seems to tick all the best-practice boxes.  Lovely long comments, single point of return, braces round every conditional, no ifs without elses.  The epitome of maintainability.

    Wait, I got it!  The problem is "fInputVal < 0.0F".  That should clearly be "0.0F > fInputVal".  You should always put the constant on the left hand side of the comparison, in case someone in the future needs to change that < to an == and then accidentally writes = instead.



  • @savar said:

     else { /* positive value */ }

     

    Instant classic

     

    I have seen a recommendation somwhere (Code Complete maybe) that you should always add (or consider adding) the else clause even if it's empty, just to show that you've considered it.  IIRC this applied to the default case in switch statements too.  This seems extreme though.



  • @savar said:

     else { /* positive value */ }

    Instant classic

     

    I agree! What kind of idiot doesn't realize that should be "non-negative".



  • @upsidedowncreature said:

    I have seen a recommendation somewhere that you should always add the else clause even if it's empty
     

    O_O



  • @too_many_usernames said:

    AbsoluteFloat() is defined in a utility file and is not declared as inline

    From The Aggregate Magic Algorithms:

    @Absolute Value of a Float said:

    IEEE floating point uses an explicit sign bit, so the absolute value can be taken by a bitwise AND with the complement of the sign bit. For IA32 32-bit, the sign bit is an int value of 0x80000000, for IA32 64-bit, the sign bit is the long long int value 0x8000000000000000. Of course, if you prefer to use just int values, the IA32 64-bit sign bit is 0x80000000 at an int address offset of +1. For example:

    double x;

    /* make x = abs(x) */
    *(((int *) &x) + 1) &= 0x7fffffff;



  • @Justice said:

    @too_many_usernames said:

    I suspect it's more like the copy-paste idea above from Oz...this codebase is the motherlode of the copy-paste anti-pattern (there is one file where essentially one function is copied I think twenty times with only a few minor changes....trouble is it works and budget/timing constraints don't allow us to refactor it.  Luckily this is just "prototype" code that is going to be replaced in the future evolve into a production system and haunt us for years.)

     

    FTFY.

    I read your later post, and ok, maybe your company is one of the few that understands what prototyping is.  Seems like for most of us, though, "prototype" is code for "half-assed brittle garbage that I threw together in a week without really knowing what I was doing and find myself still supporting after a year and a half."

    The last real 'quick prototype' I allowed myself to be tricked into doing was finally shut down about two years ago, so there's still hope.

    Now, it did have a run for about 6 years, and it survived the 'real' system by about a year and a half, but I wasn't cursed to support it until I changed employers.

    Since then, I've learned to make well-written, well-commented, extensible 'quick prototypes', which merely lack features.  This has worked much better for me - they only take a little longer to write, on average, and they save a great deal of time on the rewrite.



  • @too_many_usernames said:

    AbsoluteFloat() is defined in a utility file and is not declared as inline

    From The Aggregate Magic Algorithms:

    @Absolute Value of a Float said:

    the absolute value can be taken by a bitwise AND with the complement of the sign bit.

     

    wait, what?

     



  • 1. Take the sign bit

    2. Complement it

    3. Do a bitwise AND between the original value and the complement of the sign bit (or the other way around if you prefer)

    4. ???



  • @Zecc said:

    1. Take the sign bit

    2. Complement it

    "My, aren't you a handsome sign bit?  And a snappy dresser, too."


  • @bstorer said:

    @Zecc said:

    1. Take the sign bit

    2. Complement it

    "My, aren't you a handsome sign bit?  And a snappy dresser, too."

     Not to be confused with the english word compliment.



  • @bstorer said:

    @Zecc said:

    1. Take the sign bit

    2. Complement it

    "My, aren't you a handsome sign bit?  And a snappy dresser, too."
     

    You might need a life, although that was funny.



  • @bstorer said:

    @Zecc said:

    1. Take the sign bit

    2. Complement it

    "My, aren't you a handsome sign bit?  And a snappy dresser, too."

    That seems wrong.  Normally I have to complement my bits before they late me take them.



  • @bstorer said:

    @Zecc said:
    1. Take the sign bit
    2. Complement it
    "My, aren't you a handsome sign bit?  And a snappy dresser, too."

    Reminded me of this (last paragraph).


  • BINNED

    I'm a bit relieved about the OP's harmless WTF.

    The title made me think of such things as "nullity" again. *shudder*


Log in to reply