 Feature Articles
 CodeSOD
 Error'd
 Forums

Other Articles
 Random Article
 Other Series
 Alex's Soapbox
 Announcements
 Best of…
 Best of Email
 Best of the Sidebar
 Bring Your Own Code
 Coded Smorgasbord
 Mandatory Fun Day
 Off Topic
 Representative Line
 News Roundup
 Editor's Soapbox
 Software on the Rocks
 Souvenir Potpourri
 Sponsor Post
 Tales from the Interview
 The Daily WTF: Live
 Virtudyne
Admin
The irony is that having combed through the code and removed all the idiocies which cause the code to be slow and clumsy and fragile, there's no longer a need to replace it with something else.
Admin
This one hits too close for comfort. In the codebase I maintain, all the calculations work like this. (all that I have not yet touched, that is.)
also: use integers instead of floats. multiply by 10 to use one digit for fractions. or by 100 if resolution requires this. or divide by 10 to avoid overflow (only when you remember to do this, of course) avoid to specify units at all costs.
apologies for the rant.
Admin
No check for if OldValue is zero to then return NaN or some other indicator ?
Admin
Dunno what "decimal" is but if rounding is involved might that not go differently?
Admin
It depends on the range required. Not all languages have a 128 bit integer.
The decimal type is a power of 10 floating point number, so it doesn't suffer quite the same issues as the power of 2 floating point numbers.
Admin
It advertises percentage change and returns fractional change.
Admin
in my world of PLCsystems, an INT is a 16bit (signed) integer. to assign a process variable to that datatype is a cardinal sin. no exceptions.
Admin
I got this same issue considering what unit tests to put in place for this code. Run it with (10,20), ok, (20,10), ok, (10,10), ok, (0,10), ok, (10,0) …. Oh shirtballs!
Admin
That's one expression that would benefit from a [MethImpl(MethodImplOption.AggressiveInlining)] lol
Admin
Sometimes this is the best solution. It's the required solution in financial situationsaccounting will be coming after you with pitchforks if you round off a penny somewhere. Even without that if there's a fundamental unit that your data represents you'll have a lot less headaches if you express your values in that fundamental unit and convert for display.
Admin
Sometimes this is the best solution. It's the required solution in financial situationsaccounting will be coming after you with pitchforks if you round off a penny somewhere. Even without that if there's a fundamental unit that your data represents you'll have a lot less headaches if you express your values in that fundamental unit and convert for display.
Admin
So, nobody to point it out, that this doesn't actually return percentages, but the factor?
Admin
"James the lesser" pointed it out a day before you did.
Admin
I was going to say that the solution also doesn't check for oldValue == 0, so I would not pass this in a code review
Even if there was something upstream that checked that oldValue could not be zero or caught a div by zero, I would want a comment to indicate that this had been dealt with
Without knowing the context I would assume the old or new values could be 0
Admin
private decimal GetPercentChange(decimal oldValue, decimal newValue) => 100 * (newValue  oldValue) / oldValue;
FTFY.
Addendum 20230121 05:58: private decimal GetPercentChange(decimal oldValue, decimal newValue) => oldValue == 0 ? decimal.NaN : 100 * (newValue  oldValue) / oldValue;
FTFY.
Admin
It seems the result for negative oldvalue has changed, for example 50 to 40 is now 20% instead of +20%. I surely hope there is a comment about this somewhere, and someone has figured out what result they actually want.
For example if you do the same calculation for “profits” and “losses”, the percent change should be the same. It isn’t. The sign changes.
Admin
Good point, the original calculation was just (newValue  oldValue) / oldValue and the replacement modifies the denominator. Hopefully that was for a reason. And hopefully there aren't two domains where this is used with a possibly negative oldValue and one domain should have it work one way and the other should have it work the other way.
On the gripping hand, any domain where a negative oldValue is possible should also have to cater for an oldValue of 0, which both old and new versions die on. So maybe this is only ever used where the values are positive and the Abs() is superfluous. Or maybe it's going to explode one day when oldValue is 0. No way to tell from here.
Admin
Came upon this again from the Random Article button and actually the last two comments (including mine) are incorrect; the original wrapped the whole thing in the Abs(), so (newValue  oldValue) / Math.Abs(oldValue) is the correct replacement. If oldValue is 50 and newValue is 40 then newValue > oldValue and newValue  oldValue = 10, so the original outputs Math.Abs(10 / 50) = 0.2 and the new version outputs 10 / Math.Abs(50) = 0.2.