dotnet / dotnet/macios

Provide better API for NSTimeInterval

Open
#4,364 0 comments 3 reactions 0 assignees View on GitHub
enhancement iOS macOS request-for-comments
Dominant language
C#
Stars
2.9k
Forks
576
Avg merge
2d 13h
Merged PRs (30d)
96

Description

**Problem:**

Historically all API that use `NSTimeInterval` are converted to `System.Double`. That's fine, at least ABI wise, since it's [defined](https://developer.apple.com/documentation/foundation/nstimeinterval) as

```c
typedef double NSTimeInterval;
```

However we cannot, in .net, subclass `System.Double` so the managed type name is not helpful to convey the time scale, e.g. we can infer the following ObjC property is expressed in seconds because of the type.

```objc
@property (readonly) NSTimeInterval interval;
```

but once in C# it becomes open to interpretation because other (native) API can return `double` for times on different scales (even if, in general, the name specify that information).

```csharp
double Interval { get; }
```

https://github.com/xamarin/xamarin-macios/issues/4319 proposed the use of `TimeSpan` but that cannot be done without losing precision (since it's backed by a `long`) which is problematic in some cases.

**Proposal**

1. Use an `[Advice ("Returns an interval measured in seconds")]` attribute on properties and method that returns `NSTimeInterval`;

2. For method parameters we rename them with a `InSeconds` suffix, e.g. `SetInterval (double interval)` -> `SetInterval (double intervalInSeconds)`. See notes for existing API;

3. Write an `xtro` rule to ensure `Advice` are present and _maybe_ (see notes) for parameter names (that should be done on master, likely after we merge `xcode10` to minimize merge conflicts)

4. (optional) Update sharpie to generate the `[Advice]` and adjust the parameter names

**Notes**

* #2 is a breaking change for existing API so maybe we should only do it for new ones
* `CFTimeInterval` is also used in non-ObjC API

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.