-
Notifications
You must be signed in to change notification settings - Fork 1.2k
[EnergyFormatter] Implementation and tests #791
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
f9c22ca
Initial implementation of EnergyFormatter
43f4e92
Updates to implementation and tests to match Objective-C foundation.
e2bc6f2
Merge branch 'master' of https://github.com/kweinmeister/swift-coreli…
0920e69
Merge branch 'master' of https://github.com/kweinmeister/swift-coreli…
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,148 @@ | ||
// This source file is part of the Swift.org open source project | ||
// | ||
// Copyright (c) 2014 - 2016 Apple Inc. and the Swift project authors | ||
// Licensed under Apache License v2.0 with Runtime Library Exception | ||
// | ||
// See http://swift.org/LICENSE.txt for license information | ||
// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors | ||
// | ||
|
||
#if DEPLOYMENT_RUNTIME_OBJC || os(Linux) | ||
import Foundation | ||
import XCTest | ||
#else | ||
import SwiftFoundation | ||
import SwiftXCTest | ||
#endif | ||
|
||
class TestNSEnergyFormatter: XCTestCase { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Running these tests against the objective-c foundation give a few errors:
|
||
let formatter: EnergyFormatter = EnergyFormatter() | ||
|
||
static var allTests: [(String, (TestNSEnergyFormatter) -> () throws -> Void)] { | ||
return [ | ||
("test_stringFromJoulesJoulesRegion", test_stringFromJoulesJoulesRegion), | ||
("test_stringFromJoulesCaloriesRegion", test_stringFromJoulesCaloriesRegion), | ||
("test_stringFromJoulesCaloriesRegionFoodEnergyUse", test_stringFromJoulesCaloriesRegionFoodEnergyUse), | ||
("test_stringFromValue", test_stringFromValue), | ||
("test_unitStringFromValue", test_unitStringFromValue), | ||
("test_unitStringFromJoules", test_unitStringFromJoules) | ||
] | ||
} | ||
|
||
override func setUp() { | ||
formatter.numberFormatter.locale = Locale(identifier: "en_US") | ||
formatter.isForFoodEnergyUse = false | ||
super.setUp() | ||
} | ||
|
||
func test_stringFromJoulesJoulesRegion() { | ||
formatter.numberFormatter.locale = Locale(identifier: "de_DE") | ||
XCTAssertEqual(formatter.string(fromJoules: -100000), "-100 kJ") | ||
XCTAssertEqual(formatter.string(fromJoules: -1), "-0,001 kJ") | ||
XCTAssertEqual(formatter.string(fromJoules: 100000000), "100.000 kJ") | ||
} | ||
|
||
|
||
func test_stringFromJoulesCaloriesRegion() { | ||
XCTAssertEqual(formatter.string(fromJoules: -10000), "-2.39 kcal") | ||
XCTAssertEqual(formatter.string(fromJoules: 0.00001), "0 cal") | ||
XCTAssertEqual(formatter.string(fromJoules: 0.0001), "0 cal") | ||
XCTAssertEqual(formatter.string(fromJoules: 1), "0.239 cal") | ||
XCTAssertEqual(formatter.string(fromJoules: 10000), "2.39 kcal") | ||
} | ||
|
||
func test_stringFromJoulesCaloriesRegionFoodEnergyUse() { | ||
formatter.isForFoodEnergyUse = true | ||
XCTAssertEqual(formatter.string(fromJoules: -1), "-0 Cal") | ||
XCTAssertEqual(formatter.string(fromJoules: 0.001), "0 cal") | ||
XCTAssertEqual(formatter.string(fromJoules: 0.1), "0.024 cal") | ||
XCTAssertEqual(formatter.string(fromJoules: 1), "0.239 cal") | ||
XCTAssertEqual(formatter.string(fromJoules: 10000), "2.39 Cal") | ||
} | ||
|
||
func test_stringFromValue() { | ||
formatter.unitStyle = Formatter.UnitStyle.long | ||
XCTAssertEqual(formatter.string(fromValue: 0.002, unit: EnergyFormatter.Unit.kilojoule),"0.002 kilojoules") | ||
XCTAssertEqual(formatter.string(fromValue:0, unit:EnergyFormatter.Unit.joule), "0 joules") | ||
XCTAssertEqual(formatter.string(fromValue:1, unit:EnergyFormatter.Unit.joule), "1 joule") | ||
|
||
formatter.unitStyle = Formatter.UnitStyle.short | ||
XCTAssertEqual(formatter.string(fromValue: 0.00000001, unit:EnergyFormatter.Unit.kilocalorie), "0kcal") | ||
XCTAssertEqual(formatter.string(fromValue: 2.4, unit: EnergyFormatter.Unit.calorie), "2.4cal") | ||
XCTAssertEqual(formatter.string(fromValue: 123456, unit: EnergyFormatter.Unit.calorie), "123,456cal") | ||
|
||
formatter.unitStyle = Formatter.UnitStyle.medium | ||
formatter.isForFoodEnergyUse = true | ||
XCTAssertEqual(formatter.string(fromValue: 0.00000001, unit: EnergyFormatter.Unit.calorie), "0 cal") | ||
XCTAssertEqual(formatter.string(fromValue: 987654321, unit: EnergyFormatter.Unit.kilocalorie), "987,654,321 Cal") | ||
|
||
formatter.isForFoodEnergyUse = false | ||
XCTAssertEqual(formatter.string(fromValue: 5.3, unit: EnergyFormatter.Unit.kilocalorie), "5.3 kcal") | ||
XCTAssertEqual(formatter.string(fromValue: 873.2345, unit: EnergyFormatter.Unit.calorie), "873.234 cal") | ||
} | ||
|
||
func test_unitStringFromJoules() { | ||
var unit = EnergyFormatter.Unit.joule | ||
XCTAssertEqual(formatter.unitString(fromJoules: -100000, usedUnit: &unit), "kcal") | ||
XCTAssertEqual(unit, EnergyFormatter.Unit.kilocalorie) | ||
|
||
XCTAssertEqual(formatter.unitString(fromJoules: 0, usedUnit: &unit), "kcal") | ||
XCTAssertEqual(unit, EnergyFormatter.Unit.kilocalorie) | ||
|
||
XCTAssertEqual(formatter.unitString(fromJoules: 0.0001, usedUnit: &unit), "cal") | ||
XCTAssertEqual(unit, EnergyFormatter.Unit.calorie) | ||
|
||
XCTAssertEqual(formatter.unitString(fromJoules: 4184, usedUnit: &unit), "cal") | ||
XCTAssertEqual(unit, EnergyFormatter.Unit.calorie) | ||
|
||
XCTAssertEqual(formatter.unitString(fromJoules: 4185, usedUnit: &unit), "kcal") | ||
XCTAssertEqual(unit, EnergyFormatter.Unit.kilocalorie) | ||
|
||
XCTAssertEqual(formatter.unitString(fromJoules: 100000, usedUnit: &unit), "kcal") | ||
XCTAssertEqual(unit, EnergyFormatter.Unit.kilocalorie) | ||
|
||
formatter.numberFormatter.locale = Locale(identifier: "de_DE") | ||
XCTAssertEqual(formatter.unitString(fromJoules: -100000, usedUnit: &unit), "kJ") | ||
XCTAssertEqual(unit, EnergyFormatter.Unit.kilojoule) | ||
|
||
XCTAssertEqual(formatter.unitString(fromJoules: 0, usedUnit: &unit), "kJ") | ||
XCTAssertEqual(unit, EnergyFormatter.Unit.kilojoule) | ||
|
||
XCTAssertEqual(formatter.unitString(fromJoules: 0.0001, usedUnit: &unit), "J") | ||
XCTAssertEqual(unit, EnergyFormatter.Unit.joule) | ||
|
||
XCTAssertEqual(formatter.unitString(fromJoules: 1000, usedUnit: &unit), "J") | ||
XCTAssertEqual(unit, EnergyFormatter.Unit.joule) | ||
|
||
XCTAssertEqual(formatter.unitString(fromJoules: 1000.01, usedUnit: &unit), "kJ") | ||
XCTAssertEqual(unit, EnergyFormatter.Unit.kilojoule) | ||
} | ||
|
||
func test_unitStringFromValue() { | ||
formatter.isForFoodEnergyUse = true | ||
formatter.unitStyle = Formatter.UnitStyle.long | ||
XCTAssertEqual(formatter.unitString(fromValue: 1, unit: EnergyFormatter.Unit.kilocalorie), "Calories") | ||
XCTAssertEqual(formatter.unitString(fromValue: 2, unit: EnergyFormatter.Unit.kilocalorie), "Calories") | ||
|
||
formatter.unitStyle = Formatter.UnitStyle.medium | ||
XCTAssertEqual(formatter.unitString(fromValue: 0.00000001, unit: EnergyFormatter.Unit.kilocalorie), "Cal") | ||
XCTAssertEqual(formatter.unitString(fromValue: 987654321, unit: EnergyFormatter.Unit.kilocalorie), "Cal") | ||
|
||
formatter.unitStyle = Formatter.UnitStyle.short | ||
XCTAssertEqual(formatter.unitString(fromValue: 0.00000001, unit: EnergyFormatter.Unit.calorie), "cal") | ||
XCTAssertEqual(formatter.unitString(fromValue: 123456, unit: EnergyFormatter.Unit.kilocalorie), "C") | ||
|
||
formatter.isForFoodEnergyUse = false | ||
formatter.unitStyle = Formatter.UnitStyle.long | ||
XCTAssertEqual(formatter.unitString(fromValue: 0.002, unit: EnergyFormatter.Unit.kilojoule), "kilojoules") | ||
|
||
formatter.unitStyle = Formatter.UnitStyle.medium | ||
XCTAssertEqual(formatter.unitString(fromValue: 0.00000001, unit: EnergyFormatter.Unit.kilocalorie), "kcal") | ||
XCTAssertEqual(formatter.unitString(fromValue: 987654321, unit: EnergyFormatter.Unit.kilocalorie), "kcal") | ||
|
||
formatter.unitStyle = Formatter.UnitStyle.short | ||
XCTAssertEqual(formatter.unitString(fromValue: 0.00000001, unit: EnergyFormatter.Unit.calorie), "cal") | ||
XCTAssertEqual(formatter.unitString(fromValue: 123456, unit: EnergyFormatter.Unit.joule), "J") | ||
} | ||
|
||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is missing the meat of what
EnergyFormatter
provides on Apple platforms: namely, formatting energy values in a locale-appropriate way. On macOS:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We still need to identify a roadmap forward to handle localization bundles and the content of those for swift-corelibs-foundation. Since the environment is different (there is no "system" version of Foundation) we have to consider a methodology to store the data without relying on a common path (the bundle of the Foundation.framework). So I would say that the localization data is probably more an additional task than just the formatters. Thankfully code is mutable and we can change it after additions like this to accommodate for improvements in localizations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Certainly. In the meantime, however, is it appropriate for a formatter that is unable to localize for some particular locales and unit styles offer known incorrect output when asked, or should the code check for supported locales and otherwise bail with
NSUnimplemented()
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fallback localizations should go to en_US_POSIX so even though the current implementations may not have parity they are technically "correct" in their fallback behavior. It might be nice however to somehow have a non-failing version that would report these cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. Surely, that means that the whole thing should fall back to en_US_POSIX and not that the number should be localized and an English word appended?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In summary, the current implementation:
In writing this, I referenced
Unit.swift
, and it currently also provides English-only symbols at the moment.Shall we proceed with this initial implementation? If not, please let me know which of the above behaviors and anything else that should be changed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm ok with that, and maybe we should reflect that status in our status markdown file. Do the tests pass on macOS now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the test pass on macOS with the 2nd commit.
That's a good idea to reflect the translation status in the markdown file. It would support transparency and put more attention on resolving this technical debt.