-
-
Notifications
You must be signed in to change notification settings - Fork 6k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Replaced ChartUtils.Math
in favour of an extension on FloatingPoint
#2993
Conversation
Increases readability, and in many cases removes a set of parentheses to ensure order of operations.
Improves readability
Codecov Report
@@ Coverage Diff @@
## master #2993 +/- ##
==========================================
+ Coverage 19.41% 19.45% +0.04%
==========================================
Files 113 113
Lines 16064 16044 -20
Branches 247 247
==========================================
+ Hits 3119 3122 +3
+ Misses 12907 12884 -23
Partials 38 38
Continue to review full report at Codecov.
|
@liuxuan30 @petester42 This is a quick and easy one. |
/// NOTE: Value must be in degrees | ||
var normalizedAngle: Self { | ||
let angle = truncatingRemainder(dividingBy: 360) | ||
return (sign == .minus) ? angle + 360 : angle |
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.
the order is flipped. I need to refresh my mind first to see if they are equivalent. Or you already did that?
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.
They’re equivalent.
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.
made some stupid equations :P
/// NOTE: Value must be in degrees | ||
var normalizedAngle: Self { | ||
let angle = truncatingRemainder(dividingBy: 360) | ||
return (sign == .minus) ? angle + 360 : angle |
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.
made some stupid equations :P
Source/Charts/Utils/ChartUtils.swift
Outdated
@@ -16,9 +16,26 @@ import CoreGraphics | |||
import UIKit | |||
#endif | |||
|
|||
extension FloatingPoint { | |||
var deg2rad: Self { |
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.
Generally it's fine. But do you think we should follow the Camel-Case naming? like deg2Rad
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.
Up to you. I tried deg2Rad
and deg2RAD
and they both look silly to me. I also considered inRadians
and inDegrees
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.
OK. How about we keep old FDEG2RAD
to keep consitent?
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'll drop the F since it's implied as it's an extension on FloatingPoint
This is the one of the change I like most, indeed make it more swift and simple |
* 'master' of https://github.com/danielgindi/Charts: (23 commits) Update ViewPortHandler.swift (ChartsOrg#3143) add option to build demo projects unit tests on iOS (ChartsOrg#3121) Replaced relevant `ChartUtils` methods with `Double` extensions (ChartsOrg#2994) Update 4.0.0 with master (ChartsOrg#3135) Removed redundant ivars in BarLineChartViewBase (ChartsOrg#3043) fix ChartsOrg#1830. credit from ChartsOrg#2049 (ChartsOrg#2874) Makes ChartsDemo compiling again (ChartsOrg#3117) Fixed using wrong axis (Issue ChartsOrg#2257) Removed methods and properties deprecated in 1.0 (ChartsOrg#2996) for ChartsOrg#3061 revert animationUpdate() and animationEnd() not trigger crash if subclass does nothing The backing var is not necessary. (ChartsOrg#3000) Replaced `ChartUtils.Math` in favour of an extension on `FloatingPoint` (ChartsOrg#2993) Minor logic cleanup (ChartsOrg#3041) Fix a bug may cause infinite loop. (ChartsOrg#3073) for ChartsOrg#2745. chart should be weak. fileprivate -> private (ChartsOrg#3042) Removed `isKind(of:)` Removed @objc from internal properties Fixes for PR Made use of `==` where appropriate to simplify logic ... # Conflicts: # Source/Charts/Data/Implementations/Standard/LineChartDataSet.swift # Source/Charts/Renderers/BarChartRenderer.swift
Increases readability, and in many cases removes a set of parentheses to ensure order of operations. Also can be applied to any
FloatingPoint
type, not justDouble
andCGFloat