Enable NRT for classes in namespace Ical.Net.Utility #706
Conversation
axunonb
force-pushed
the
pr/nrt-for-ical-net-utility
branch
from
ac4f5e0 to
6f91d5c
Compare
| public static DateTimeZone GetZone(string tzId, bool useLocalIfNotFound = true) | ||
| { | ||
| var exMsg = $"Unrecognized time zone id {tzId}"; | ||
|
|
||
| if (string.IsNullOrWhiteSpace(tzId)) |
There was a problem hiding this comment.
We removed the implicit usage of the local time zone in other parts, didn't we? Hope, this is the only one left.
| @@ -634,7 +634,7 @@ public int CompareTo(IDateTime? dt) | |||
| public string ToString(string? format, IFormatProvider? formatProvider) | |||
| { | |||
| formatProvider ??= CultureInfo.InvariantCulture; | |||
| var dateTimeOffset = DateUtil.ToZonedDateTimeLeniently(Value, _tzId).ToDateTimeOffset(); | |||
| var dateTimeOffset = DateUtil.ToZonedDateTimeLeniently(Value, _tzId ?? string.Empty).ToDateTimeOffset(); | |||
There was a problem hiding this comment.
Shouldn't we handle floating separately? Not sure how the method behaves if we pass an empty string.
| @@ -66,18 +62,21 @@ public static bool Equals<T>(IEnumerable<T> left, IEnumerable<T> right, bool ord | |||
| return left.SequenceEqual(right); | |||
| } | |||
|
|
|||
| // No multiple enumerations | |||
| var rightArray = right as T[] ?? right.ToArray(); | |||
There was a problem hiding this comment.
Eager iteration would only help if right/left would be generators, which doesn't seem to be the case in any of today's uses, otherwise it would rather add overhead.
| } | ||
| catch (Exception) | ||
| { | ||
| //It's not possible to sort some collections of things (like Calendars) in any meaningful way. Properties can be null, and there's no natural | ||
| //ordering for the contents therein. In cases like that, the best we can do is treat them like sets, and compare them. We don't maintain | ||
| //fidelity with respect to duplicates, but it seems better than doing nothing | ||
| var leftSet = new HashSet<T>(left); | ||
| var rightSet = new HashSet<T>(right); | ||
| var leftSet = new HashSet<T>(leftArray); |
There was a problem hiding this comment.
I think this whole way of comparing isn't too helpful. We discussed this in another ticket, that Equals might not be too helpful for many of the types, e.g. Calendar. But that's a topic for a different PR.
There was a problem hiding this comment.
Yes, I wanted to stick to NRT
|
No description provided.