Distance formatter fixes #3331 #7585
Conversation
frederoni
added
iOS
|
@frederoni, thanks for your PR! By analyzing this pull request, we identified @1ec5, @boundsj and @incanus to be potential reviewers. |
| XCTAssertEqualObjects([formatter stringFromDistance:3218.68], [self testStringWithDistance:2 formatter:formatter unit:@"miles"]); | ||
| } | ||
|
|
||
| - (NSString *)testStringWithDistance:(CLLocationDistance)distance formatter:(MGLDistanceFormatter *)formatter unit:(NSString *)unit { |
There was a problem hiding this comment.
In XCTest, I think we should avoid writing utility methods that start with test.
There was a problem hiding this comment.
It only recognizes methods prefixed with test if the return type is void but I agree.
|
It looks like you’ve completely reverse engineered NSLengthFormatter with some adjustments for distance-based measurements. That’s an admirable accomplishment. 👏 However, I think we should try to make this implementation more maintainable by leveraging an NSLengthFormatter internally and tweaking its output as needed. That would remove the need for hard-coded conversions and most localizable strings. NSLengthFormatter’s behavior is almost exactly what we want. The only two differences are that we want distance-based rounding behavior, and a fractional mileage should be displayed in miles (ideally as a vulgar fraction or mixed number of miles by default) instead of yards. Besides maintainability, another important benefit of NSLengthFormatter is that it always matches the user’s system language and region settings, even if the application doesn’t. As written, MGLDistanceFormatter could potentially output something inconsistent like “૨૬ meters” based on the user’s system language. For comparison, the Swift implementation mentioned in #3331 (comment) weighs in at only 110 lines, including an “elaborate hack” that uses vulgar fractions for U.S. mileages while tiptoeing around any potential localization issues. Without that hack, it would be a mere 40 lines. |
|
Closing in favor of #7592 |
Fixes #3331
This PR adds a formatter that is meant to be used for geographic distances. (i.e. #7432)
It supports localization and pluralization. By default, it uses the user‘s current locale and opts for feet over yards unlike NSLengthFormatter.