Forgetting the Sign on the Change
A few days ago, I wrote a few static methods on a class that could be used in the testing of changed values. They aren't anything special, but it's something that I know from experience you need to do right, or it's going to be painful, and in order to do it right, you really need to handle all the edge conditions. Normally, this would be in-line code for another developer, but I've learned that it really clutters your code to do it right. Hence, the static methods.
My first cut was:
double Instrument::pctChange( double oldValue, double newValue ) { double chg = NAN; if (!::isnan(oldValue) && !::isnan(newValue)) { if (fabs(oldValue) < 1.0e-6) { chg = 1.0e6; } else { chg = (newValue - oldValue)*100.0/oldValue; } } return chg; } double Instrument::pctChange( int64_t oldValue, int64_t newValue ) { double chg = NAN; if (!::isnan(oldValue) && !::isnan(newValue)) { if (oldValue == 0) { chg = 1.0e6; } else { chg = (newValue - oldValue)*100.0/oldValue; } } return chg; }
Having one that takes doubles and another that takes int64_t is nice in that we store the floating point numbers as integers internally to make math faster and transmission far faster.
I then proceeded to use this in the test of whether or not on instrument needed to be calculated based on the changes in some values of that instrument. The code was pretty simple, but it compounded problems int he above code even more:
double spotChg = pctChg(res.spot, getSpotAsInt()); … if ((maxChg > 0.0) && !::isnan(spotChg) && (spotChg < maxChg)) { … }
It was pretty easy to see, when I got word that this code wasn't working, that the problem was in the loss of the sign in the test. I needed the absolute value of the percent change in the test, not just the value itself. All negative changes were being discarded because they passed this test. Not good.
So I decided to incorporate the absolute value into the method, and at the same time, fix up a few problems I saw with the methods themselves:
double Instrument::pctChange( double oldValue, double newValue ) { double chg = NAN; if (!::isnan(oldValue) && !::isnan(newValue)) { if (fabs(newValue - oldValue) < 1.0e-8) { chg = 0.0; } else if (fabs(oldValue) < 1.0e-6) { chg = (newValue > 0.0 ? 1.0e6 : -1.0e6); } else { chg = (newValue - oldValue)*100.0/oldValue; } } return chg; } double Instrument::pctChange( int64_t oldValue, int64_t newValue ) { double chg = NAN; if (!::isnan(oldValue) && !::isnan(newValue)) { if (newValue == oldValue) { chg = 0.0; } else if (oldValue == 0) { chg = (newValue > 0 ? 1.0e6 : -1.0e6); } else { chg = (newValue - oldValue)*100.0/oldValue; } } return chg; } double Instrument::absPctChange( double oldValue, double newValue ) { return fabs(pctChg(oldValue, newValue)); } double Instrument::absPctChange( int64_t oldValue, int64_t newValue ) { return fabs(pctChg(oldValue, newValue)); }
and then in my code I simply used the correct form of the method:
double spotChg = absPctChg(res.spot, getSpotAsInt()); … if ((maxChg > 0.0) && !::isnan(spotChg) && (spotChg < maxChg)) { … }
and we should be good to go.
The method now properly detects no change - regardless of the old value being zero, and the sign of the very large change is in accordance with the sign on the new value. Also, the absolute value being backed into the other methods makes it very easy not to forget this going forward.