When Math Attacks



  • I just came across this gem in a "DataHelper" class that is used to hold static data related methods for an entire application. When I confronted the apparent owner of the code of it's intended purpose, I eventually got the "I didn't write it, I just moved it because I thought it might be used elsewhere" reasoning.

    /// <summary>
    /// Rounding function with Ceiling enforced
    /// </summary>
    /// <param name="d">The decimal to round off</param>
    /// <returns></returns>
    public static decimal Round(decimal d)
    {
        decimal ret = Decimal.Round(d, 1);

        if (ret == d - 0.05m)
        {
        return ret + 0.1m;
        }
        else
        {
        return ret;
        }
    }

     

    I have since replaced it with:

     

    public static decimal Ceiling(decimal Number, int Decimals)
    {
        return Math.Ceiling(Number * (10 ^ Decimals)) / (10 ^ Decimals);
    }

     

    Here's the sweet conversation that happened between us:

    DigitallyBorn: Hey, what's the point of the DataHelper.Round() method?

    Co-Worker: it has some rounding specifically used in some report

    DigitallyBorn: what's special about it?

    Co-Worker: u mean why not use math.round instead?

    DigitallyBorn: it's named generically and in a generic place, but doesn't really describe what it's for
    yeah .. what does it do differently?

    Co-Worker: I jst moved it from a report, cz it seemed generic

    DigitallyBorn: do you know what it does?

    Co-Worker: lemme see the code
    Its got summary- Rounding function with cieling enforced

    DigitallyBorn: yeah, but I don't know what that means

    Co-Worker: Well,.. it rounds it to nearest 0.1
    no, it rounds it to +0.1 always

    DigitallyBorn: I don't get it

    Co-Worker: i didnt really think that much before moving it.. seems like it is report specific
    but i tht it might be useful in other reports
    - rephrase: it might have to be used in other reports

    DigitallyBorn: it's not even really a correct rounding

    Co-Worker: DataHelper.Round(0.11) = 0.2
    Round(0.11) = 0.1
    That was needed for some issue

    DigitallyBorn: you could've just done Math.Ceiling(myNumber * 100) / 100

    Co-Worker: I didnt write that function
    i just moved it from some report
    for design considerations

    DigitallyBorn: it probably doesn't belong there now that we agree it's not generic .. or, at the very least, it needs to be named more appropriately

    Co-Worker: makes sense



  • What language uses ^ as a power operator? It looks like C# to me, but ^ is the bitwise xor operator on ints in C#...



  • @Welbog said:

    What language uses ^ as a power operator? It looks like C# to me, but ^ is the bitwise xor operator on ints in C#...

    You're quite right. It's c#, and it's wrong. Perhaps I should post AFTER I've gone through testing my code. 



  • If I'm not mistaken, VB still uses the ^ for power. That would explain why some guys moving from vb to c# might try to use ^ to represent a power instead of math.pow..



  • you sound like a knob! i bet when your co-worker said "makes sense" he was thinking "now go away and tell someone else how brillant your are"

     

    you should lurk moar.



  • So you replaced working code with non-working code, and then posted about it here?

    I think the real WTF is apparent.



  • @username_not_found said:

    So you replaced working code with non-working code, and then posted about it here?

    I think the real WTF is apparent.

    More like, I posted before I had finished developing the fix. I hadn't yet committed any of my changes and was still working on it when the first reply came in. But I will cop to a WTF myself. My first post on WTF .. I was excited .. I jumped the gun. I am shamed.



  • @bobday said:

    you sound like a knob! i bet when your co-worker said "makes sense" he was thinking "now go away and tell someone else how brillant your are"

     

    you should lurk moar.

    Yeah, I am a complete a-hole to him. It's because the rest of the team and I have run into many examples of his confusing or long-winded code. It's been quite a while of my frustrations building up .. I should place nicer.

    And, even though he says he didn't write that code, he actually did.
     



  • By the way, the working code is:

    public static decimal Ceiling(decimal Number, int Decimals)
    {
        decimal l_DecimalShift = Convert.ToDecimal(Math.Pow(10, Decimals));
        return Math.Ceiling(Number * l_DecimalShift) / l_DecimalShift;
    }



  • @DigitallyBorn said:

    When I confronted the apparent owner of the code of it's intended purpose, I eventually got the "I didn't write it, I just moved it because I thought it might be used elsewhere" reasoning.

    This is the true WTF. People who do this create a massive ball-of-mud dumping ground of little generic functions that are inevitably worse than the built-in equivalents. Ultimately, this "general library" has a gigantic library of crapola, 99% of which only exists to stroke the authors egos, and which is so large it's unwieldy, unnavigable and basically useless. Duplicate functions become increasingly common. Occasionally some new programmer will write something, and the onlooking weenie programmer will say, "Oh, don't do that yourself, there's a library function for that" followed by, in a barely audible whisper, "which I wrote, tee-hee."

    The cure for this is to disallow in-house general libraries. Everyone thinks they're needed, but they really aren't. They do far more harm than good. If there's really a need for some in-house library, give it a name other than "general" or "misc" or "utils" or "shared" or "common." If you can't come up with a more meaningful name, it's a sure sign the functions need to be printed out and burned in effigy.

    I don't know about .Net, but in Java, one of the most common manifestations of this is a GridBagLayout "helper." Another is a "generic table model."



  • You're final answer is still a WTF.  The Decimal datatype (in c#) is explicitly there due to how it handles rounding.  Specifically, it is there to avoid the type of crap you wrote.

    Most rounding problems occur because people misuse the Double data type.  With double's you will see rounding errors depending on the math involved; and the return call will actually force the compiler to take a decimal, convert it to a double, perform the division, then convert it back to a decimal. 

    Having programmed calculation functions for the mortgage industry, believe me I know where the problems lie.

    Honestly, the function should have been completely removed in favor of the BUILT IN rounding functions of Decimal.  So, hat's off to you oh great one that recodes built in functions.



  • VGR, I completely agree. The original purpose of our "DataHelper" was to hold the methods to handle checking for DBNull (returned from databases) and translating that to c#'s null. To do this, there were only 2 static methods with a dozen overloads of each. It really should've been named something more specific. We've done well in that of the 275+ lines of code in this 'helper' class, there are only 50 that aren't serving the original purpose.



  • @clively said:

    You're final answer is still a WTF.  The Decimal datatype (in c#) is explicitly there due to how it handles rounding.  Specifically, it is there to avoid the type of crap you wrote.

    Most rounding problems occur because people misuse the Double data type.  With double's you will see rounding errors depending on the math involved; and the return call will actually force the compiler to take a decimal, convert it to a double, perform the division, then convert it back to a decimal. 

    Having programmed calculation functions for the mortgage industry, believe me I know where the problems lie.

    Honestly, the function should have been completely removed in favor of the BUILT IN rounding functions of Decimal.  So, hat's off to you oh great one that recodes built in functions.

    Can you explain a little more? Ultimately, I am looking to get a ceiling value of a decimal place. If there is a better or built-in way, I would much rather use that.

    For example, I should be able to get 16.2 from 16.11.

    So: DataHelper.Ceiling(16.11, 1) = 16.2 



  • @DigitallyBorn said:

    Can you explain a little more? Ultimately, I am looking to get a ceiling value of a decimal place. If there is a better or built-in way, I would much rather use that.

    For example, I should be able to get 16.2 from 16.11.

    So: DataHelper.Ceiling(16.11, 1) = 16.2 

    <hints id="hah_hints"></hints>

    Read the documentation for [B]Decimal.Round([I]Decimal, Int32[/I])[/B] or [B]Decimal.Round([I]Decimal, Int32, MidpointRounding[/I])[/B].

    Or just use [B]Math.Round[/B] which handles Decimals anyway.

    Edit: Forgot while I was writing this that you wanted Ceiling as opposed to Rounding.  No matter though, just add 0.05 to the operand.



  • Your code is very wrong. It very rarely matches the output of the previous code. That code was a workaround to the default rounding mode of the decimal type in .net (ToEven). It finds the case where the midpoint (.05) is rounded down due to the closest even number being less than the number being rounded.

    public static decimal Round(decimal d)
    {
         return Decimal.Round(d, 1, MidpointRounding.AwayFromZero);
    }

    would match the output for positive decimals, but would round  .05 down if the number is negative. If you add a small increment such as 1e-24 (chosen randomly for nor good reason) like so:

    public static decimal Round(decimal d)
    {
         return Decimal.Round(d + 1e-24m, 1, MidpointRounding.AwayFromZero);
    }

    It should match for all numbers (depending on the precision maintained )



  • @Welbog said:

    What language uses ^ as a power operator?

    MATLAB and relatives.



  • @clively said:

    Most rounding problems occur because people misuse the Double data type.

    More precisely: most rounding problems occur because people have no clue what floating point is or why you might want to use it, let alone how to use it.

    If you are dealing with money, you do not want to use floating point. Ever. Floating point is designed for physics. The floating point arithmetic operators do not behave like the integer arithmetic operators that you know.



  • @asuffield said:

    @clively said:

    Most rounding problems occur because people misuse the Double data type.

    More precisely: most rounding problems occur because people have no clue what floating point is or why you might want to use it, let alone how to use it.

    If you are dealing with money, you do not want to use floating point. Ever. Floating point is designed for physics. The floating point arithmetic operators do not behave like the integer arithmetic operators that you know.

    I've always had the feeling that floating point numbers are the variable type equivalent of Visual Basic.

    They are very good tools if you know how to use them, but it is too easy to use them without thinking.

    But then again - I am doing embedded signal processing, and have no use for the exponential accuracy of floats.



  • @olm said:

    I've always had the feeling that floating point numbers are the variable type equivalent of Visual Basic.

    They are very good tools if you know how to use them, but it is too easy to use them without thinking.

    It's more that they are probably the wrong tool for your job, unless you're doing certain specialised things. They are not a hammer to be applied to any problem that involves bashing things.

    The problem is that CPUs implement integer and floating point math, because they are fundamentally different things, and people then assume that if you want more than integer precision, you should use floating point. The reality is that you should 'normally' use fixed point maths on the integer logic, and leave the FPU alone, unless you actually know what it does, why it's different to integer math, and why your specialised problem needs it. This confusion has been furthered by the lack of a fixed point implementation in the C standard library, even though most other languages have one.



  • @Aaron said:

    ed to Rounding.  No matter though, just add 0.05 to the operand.

    AaronCeil(1.0) == Round(1.05,1) == 1.1

    ooops 


Log in to reply