Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

NSDate(MGLAdditions) turns seconds into nanoseconds #8264

Closed
1ec5 opened this issue Mar 3, 2017 · 3 comments
Closed

NSDate(MGLAdditions) turns seconds into nanoseconds #8264

1ec5 opened this issue Mar 3, 2017 · 3 comments
Assignees
Labels
bug iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS
Milestone

Comments

@1ec5
Copy link
Contributor

1ec5 commented Mar 3, 2017

MGLDurationInSecondsFromTimeInterval() converts an NSTimeInterval (measured in seconds) into an mbgl::Duration (measured in nanoseconds), but MGLTimeIntervalFromDurationInSeconds() reinterprets an mbgl::Duration (measured in nanoseconds) as an NSTimeInterval without changing its value. As a result, these functions don’t round-trip and -[MGLStyle transitionDuration] and -[MGLStyle transitionDelay] return the wrong values. We should fix MGLTimeIntervalFromDurationInSeconds() to convert back into seconds. This page has examples for working with duration. We should also add a unit test to ensure round-tripping.

Additionally, the functions’ names incorrectly imply that mbgl::Duration is measured in seconds. They should be renamed MGLDurationFromTimeInterval() and MGLTimeIntervalFromDuration(), respectively.

Finally, #5980 declared and defined these functions within @interface and @implementation blocks, as though they were category methods, but they aren’t even methods in the first place. The NSDate(MGLAdditions) category should be removed, because these functions have nothing to do with NSDate. (These functions should remain in NSDate+MGLAdditions.h, because they relate to NSTimeInterval, which is declared in NSDate.h.)

/cc @fabian-guerra @incanus

@1ec5 1ec5 added bug iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS labels Mar 3, 2017
@1ec5 1ec5 added this to the ios-v3.5.0 milestone Mar 3, 2017
@1ec5
Copy link
Contributor Author

1ec5 commented Mar 3, 2017

MGLDurationInSecondsFromTimeInterval() has quite a bit of history:

Not to point any blame – the naming made sense at the time, but it’s a good thing that we can finally remove “in seconds”!

@jfirebaugh
Copy link
Contributor

#8276 addressed the above issues but introduced others (https://github.com/mapbox/mapbox-gl-native/pull/8276/files#r104329402). Keeping this open until those are resolved.

@jfirebaugh
Copy link
Contributor

Fixed in #8290.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS
Projects
None yet
Development

No branches or pull requests

3 participants