AlexDenisov / iActiveRecord

ActiveRecord for iOS without CoreData, only SQLite
http://alexdenisov.github.com/iActiveRecord/
MIT License
354 stars 50 forks source link

Fix NSDecimalNumber precision on saving to sqlite #28

Closed mgod closed 11 years ago

mgod commented 11 years ago

We're using NSDecimalNumber to save currency values, and the current conversion often adds trailing imprecisions in the numbers on save. For example:

NSDecimalNumber *test = [NSDecimalNumber decimalNumberWithMantissa:1123563 exponent:-3 isNegative:NO];
NSLog(@"number converts: %@", [test stringValue]);
NSLog(@"number converts: %f", [test floatValue]);

Output:

2013-04-11 10:03:46.934 Splitwise[5028:c07] number converts: 1123.563
2013-04-11 10:03:46.934 Splitwise[5028:c07] number converts: 1123.562988

Using stringValue on NSDecimalNumber is stable for all locales I've tested on (it uses descriptionWithLocale: internally) and converts to something sqlite can deal with. I went ahead and forced en_US_POSIX in the patch, even though I've yet to see an issue with different locales.

The downside of this approach is that there are values that cannot be stored in sqlite that can be created with stringValue. Currently, I believe these will silently translate into incorrect float values, but this might cause crashes now instead.

WARNING: I've been unable to get the unit tests to run. I'd love to get these working, but I can't figure out which target is supposed to run tests or how that has been set up. My test builds, but I haven't been able to confirm that it exposes the problem before the fix.

AlexDenisov commented 11 years ago

I use Cedar for UnitTests and CocoaPods for dependency management. Follow this steps to run tests

[sudo] gem install cocoapods
pod setup
cd project_dir
pod install
open iActiveRecord.xcworkspace

Then build & run UnitTests target.

AlexDenisov commented 11 years ago

I ran tests on my machine and they all have passed. Please, fix typo and I'll merge this one.

P.S. could you try to use primitive types (float or double) instead of NSDecimalNumber?

mgod commented 11 years ago

Thanks for the follow-up. Typo fixed. Unit tests passed except for SaveUpdateSpec Update should save/load record with primitive types.

We are using NSDecimalNumber because we have to do bankers math on a lot of our data, which needs precise base-10 math you can't get easily from other data types. I suspect once you get your primitive types working it would be a good idea to switch NSDecimalNumber to saving as a string in sqlite (you'd lose the ability to do sql comparisons, but it would be completely accurate, which is the real point of dealing with the hassles of NSDecimalNumber).