From 86ecb64d27d09c67480f3684368bfbc63e854d3e Mon Sep 17 00:00:00 2001 From: Markus Minichmayr Date: Mon, 2 Dec 2024 15:52:00 +0100 Subject: [PATCH 1/5] Make `RucrrencePattern.Count` nullable to avoid using magic number `int.MinValue`. --- Ical.Net/DataTypes/RecurrencePattern.cs | 4 +-- .../Evaluation/RecurrencePatternEvaluator.cs | 10 ++++--- Ical.Net/Evaluation/TodoEvaluator.cs | 2 +- .../DataTypes/RecurrencePatternSerializer.cs | 26 ++++++++++++++++++- 4 files changed, 35 insertions(+), 7 deletions(-) diff --git a/Ical.Net/DataTypes/RecurrencePattern.cs b/Ical.Net/DataTypes/RecurrencePattern.cs index da77b738b..da1ca7555 100644 --- a/Ical.Net/DataTypes/RecurrencePattern.cs +++ b/Ical.Net/DataTypes/RecurrencePattern.cs @@ -29,7 +29,7 @@ public class RecurrencePattern : EncodableDataType public DateTime? Until { get; set; } - public int Count { get; set; } = int.MinValue; + public int? Count { get; set; } /// /// Specifies how often the recurrence should repeat. @@ -181,7 +181,7 @@ public override int GetHashCode() #pragma warning restore 0618 hashCode = (hashCode * 397) ^ (int) Frequency; hashCode = (hashCode * 397) ^ Until.GetHashCode(); - hashCode = (hashCode * 397) ^ Count; + hashCode = (hashCode * 397) ^ (Count ?? 0); hashCode = (hashCode * 397) ^ (int) FirstDayOfWeek; hashCode = (hashCode * 397) ^ CollectionHelpers.GetHashCode(BySecond); hashCode = (hashCode * 397) ^ CollectionHelpers.GetHashCode(ByMinute); diff --git a/Ical.Net/Evaluation/RecurrencePatternEvaluator.cs b/Ical.Net/Evaluation/RecurrencePatternEvaluator.cs index 82f9e3071..aedd594bd 100644 --- a/Ical.Net/Evaluation/RecurrencePatternEvaluator.cs +++ b/Ical.Net/Evaluation/RecurrencePatternEvaluator.cs @@ -221,7 +221,7 @@ private IEnumerable GetDates(IDateTime seed, DateTime? periodStart, Da // optimize the start time for selecting candidates // (only applicable where a COUNT is not specified) - if (pattern.Count == int.MinValue) + if (pattern.Count is null) { var incremented = seedCopy; while (incremented < periodStart) @@ -229,6 +229,10 @@ private IEnumerable GetDates(IDateTime seed, DateTime? periodStart, Da seedCopy = incremented; IncrementDate(ref incremented, pattern, pattern.Interval); } + } else + { + if (pattern.Count < 1) + throw new Exception("Count must be greater than 0"); } // Do the enumeration in a separate method, as it is a generator method that is @@ -256,7 +260,7 @@ private IEnumerable EnumerateDates(DateTime originalDate, DateTime see break; } - if (pattern.Count >= 1 && dateCount >= pattern.Count) + if (dateCount >= pattern.Count) { break; } @@ -281,7 +285,7 @@ private IEnumerable EnumerateDates(DateTime originalDate, DateTime see // from the previous year. // // exclude candidates that start at the same moment as periodEnd if the period is a range but keep them if targeting a specific moment - if (pattern.Count >= 1 && dateCount >= pattern.Count) + if (dateCount >= pattern.Count) { break; } diff --git a/Ical.Net/Evaluation/TodoEvaluator.cs b/Ical.Net/Evaluation/TodoEvaluator.cs index 756ca8648..d7d9ddd5c 100644 --- a/Ical.Net/Evaluation/TodoEvaluator.cs +++ b/Ical.Net/Evaluation/TodoEvaluator.cs @@ -65,7 +65,7 @@ private void DetermineStartingRecurrence(PeriodList rdate, ref IDateTime referen private void DetermineStartingRecurrence(RecurrencePattern recur, ref IDateTime referenceDateTime) { - if (recur.Count != int.MinValue) + if (recur.Count.HasValue) { referenceDateTime = Todo.Start.Copy(); } diff --git a/Ical.Net/Serialization/DataTypes/RecurrencePatternSerializer.cs b/Ical.Net/Serialization/DataTypes/RecurrencePatternSerializer.cs index 647a9b1e4..16ae3e722 100644 --- a/Ical.Net/Serialization/DataTypes/RecurrencePatternSerializer.cs +++ b/Ical.Net/Serialization/DataTypes/RecurrencePatternSerializer.cs @@ -58,12 +58,27 @@ public virtual void CheckRange(string name, IList values, int min, int max) } } + public virtual void CheckRange(string name, IList values, int min, int max) + { + var allowZero = (min == 0 || max == 0); + foreach (var value in values) + { + CheckRange(name, value, min, max, allowZero); + } + } + public virtual void CheckRange(string name, int value, int min, int max) { var allowZero = min == 0 || max == 0; CheckRange(name, value, min, max, allowZero); } + public virtual void CheckRange(string name, int? value, int min, int max) + { + var allowZero = min == 0 || max == 0; + CheckRange(name, value, min, max, allowZero); + } + public virtual void CheckRange(string name, int value, int min, int max, bool allowZero) { if (value != int.MinValue && (value < min || value > max || (!allowZero && value == 0))) @@ -73,6 +88,15 @@ public virtual void CheckRange(string name, int value, int min, int max, bool al } } + public virtual void CheckRange(string name, int? value, int min, int max, bool allowZero) + { + if (value != null && (value < min || value > max || (!allowZero && value == 0))) + { + throw new ArgumentException(name + " value " + value + " is out of range. Valid values are between " + min + " and " + max + + (allowZero ? "" : ", excluding zero (0)") + "."); + } + } + public virtual void CheckMutuallyExclusive(string name1, string name2, T obj1, TU obj2) { if (Equals(obj1, default(T)) || Equals(obj2, default(TU))) @@ -160,7 +184,7 @@ public override string SerializeToString(object obj) values.Add("WKST=" + Enum.GetName(typeof(DayOfWeek), recur.FirstDayOfWeek).ToUpper().Substring(0, 2)); } - if (recur.Count != int.MinValue) + if (recur.Count.HasValue) { values.Add("COUNT=" + recur.Count); } From ce089b148e27d694efb1db4130a8cf20a4076c35 Mon Sep 17 00:00:00 2001 From: Markus Minichmayr Date: Mon, 2 Dec 2024 15:57:56 +0100 Subject: [PATCH 2/5] Make `RucrrencePattern.Interval` nullable to avoid using magic number `int.MinValue`. --- Ical.Net/DataTypes/RecurrencePattern.cs | 6 ++---- .../Serialization/DataTypes/RecurrencePatternSerializer.cs | 5 ----- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/Ical.Net/DataTypes/RecurrencePattern.cs b/Ical.Net/DataTypes/RecurrencePattern.cs index da1ca7555..b0ba1a1c4 100644 --- a/Ical.Net/DataTypes/RecurrencePattern.cs +++ b/Ical.Net/DataTypes/RecurrencePattern.cs @@ -20,7 +20,7 @@ namespace Ical.Net.DataTypes; /// public class RecurrencePattern : EncodableDataType { - private int _interval = int.MinValue; + private int? _interval; #pragma warning disable 0618 private RecurrenceRestrictionType? _restrictionType; private RecurrenceEvaluationModeType? _evaluationMode; @@ -39,9 +39,7 @@ public class RecurrencePattern : EncodableDataType /// public int Interval { - get => _interval == int.MinValue - ? 1 - : _interval; + get => _interval ?? 1; set => _interval = value; } diff --git a/Ical.Net/Serialization/DataTypes/RecurrencePatternSerializer.cs b/Ical.Net/Serialization/DataTypes/RecurrencePatternSerializer.cs index 16ae3e722..4a6b1814e 100644 --- a/Ical.Net/Serialization/DataTypes/RecurrencePatternSerializer.cs +++ b/Ical.Net/Serialization/DataTypes/RecurrencePatternSerializer.cs @@ -157,11 +157,6 @@ public override string SerializeToString(object obj) //every week for a WEEKLY rule, every month for a MONTHLY rule and //every year for a YEARLY rule. var interval = recur.Interval; - if (interval == int.MinValue) - { - interval = 1; - } - if (interval != 1) { values.Add("INTERVAL=" + interval); From 996ff2b0d4db390435a1977bbb43971f6241bebf Mon Sep 17 00:00:00 2001 From: Markus Minichmayr Date: Mon, 2 Dec 2024 16:06:59 +0100 Subject: [PATCH 3/5] Make `WeekDay.Offset` nullable to avoid using `int.MinValue` as magic number. --- Ical.Net/Constants.cs | 3 +-- Ical.Net/DataTypes/WeekDay.cs | 13 ++++++------- Ical.Net/Evaluation/RecurrencePatternEvaluator.cs | 8 ++++---- .../DataTypes/RecurrencePatternSerializer.cs | 7 ++++--- .../Serialization/DataTypes/WeekDaySerializer.cs | 4 ++-- 5 files changed, 17 insertions(+), 18 deletions(-) diff --git a/Ical.Net/Constants.cs b/Ical.Net/Constants.cs index 7557dccd2..6a99e831e 100644 --- a/Ical.Net/Constants.cs +++ b/Ical.Net/Constants.cs @@ -211,7 +211,6 @@ public enum FrequencyType /// public enum FrequencyOccurrence { - None = int.MinValue, Last = -1, SecondToLast = -2, ThirdToLast = -3, @@ -374,4 +373,4 @@ public static class RegexDefaults /// The default timeout for regular expressions. /// public static readonly TimeSpan Timeout = TimeSpan.FromMilliseconds(TimeoutMilliseconds); -} \ No newline at end of file +} diff --git a/Ical.Net/DataTypes/WeekDay.cs b/Ical.Net/DataTypes/WeekDay.cs index eb59a3eaa..a50d451a4 100644 --- a/Ical.Net/DataTypes/WeekDay.cs +++ b/Ical.Net/DataTypes/WeekDay.cs @@ -4,6 +4,7 @@ // using System; +using System.Collections.Generic; using System.IO; using Ical.Net.Serialization.DataTypes; @@ -14,14 +15,12 @@ namespace Ical.Net.DataTypes; /// public class WeekDay : EncodableDataType { - public virtual int Offset { get; set; } = int.MinValue; + public virtual int? Offset { get; set; } public virtual DayOfWeek DayOfWeek { get; set; } public WeekDay() - { - Offset = int.MinValue; - } + { } public WeekDay(DayOfWeek day) : this() { @@ -52,7 +51,7 @@ public override bool Equals(object obj) return ds.Offset == Offset && ds.DayOfWeek == DayOfWeek; } - public override int GetHashCode() => Offset.GetHashCode() ^ DayOfWeek.GetHashCode(); + public override int GetHashCode() => (Offset ?? 0).GetHashCode() ^ DayOfWeek.GetHashCode(); /// public override void CopyFrom(ICopyable obj) @@ -81,8 +80,8 @@ public int CompareTo(object obj) var compare = DayOfWeek.CompareTo(weekday.DayOfWeek); if (compare == 0) { - compare = Offset.CompareTo(weekday.Offset); + compare = Comparer.Default.Compare(Offset, weekday.Offset); } return compare; } -} \ No newline at end of file +} diff --git a/Ical.Net/Evaluation/RecurrencePatternEvaluator.cs b/Ical.Net/Evaluation/RecurrencePatternEvaluator.cs index aedd594bd..25993a260 100644 --- a/Ical.Net/Evaluation/RecurrencePatternEvaluator.cs +++ b/Ical.Net/Evaluation/RecurrencePatternEvaluator.cs @@ -751,9 +751,9 @@ private static int GetWeekDayOffset(DateTime date, DayOfWeek startOfWeek) /// /// The list from which to extract the element. /// The position of the element to extract. - private List GetOffsetDates(List dates, int offset) + private List GetOffsetDates(List dates, int? offset) { - if (offset == int.MinValue) + if (offset is null) { return dates; } @@ -762,11 +762,11 @@ private List GetOffsetDates(List dates, int offset) var size = dates.Count; if (offset < 0 && offset >= -size) { - offsetDates.Add(dates[size + offset]); + offsetDates.Add(dates[size + offset.Value]); } else if (offset > 0 && offset <= size) { - offsetDates.Add(dates[offset - 1]); + offsetDates.Add(dates[offset.Value - 1]); } return offsetDates; } diff --git a/Ical.Net/Serialization/DataTypes/RecurrencePatternSerializer.cs b/Ical.Net/Serialization/DataTypes/RecurrencePatternSerializer.cs index 4a6b1814e..343b60dc3 100644 --- a/Ical.Net/Serialization/DataTypes/RecurrencePatternSerializer.cs +++ b/Ical.Net/Serialization/DataTypes/RecurrencePatternSerializer.cs @@ -81,7 +81,7 @@ public virtual void CheckRange(string name, int? value, int min, int max) public virtual void CheckRange(string name, int value, int min, int max, bool allowZero) { - if (value != int.MinValue && (value < min || value > max || (!allowZero && value == 0))) + if ((value < min || value > max || (!allowZero && value == 0))) { throw new ArgumentException(name + " value " + value + " is out of range. Valid values are between " + min + " and " + max + (allowZero ? "" : ", excluding zero (0)") + "."); @@ -429,11 +429,12 @@ public override object Deserialize(TextReader tr) } else if ((match = RelativeDaysOfWeek.Match(item)).Success) { - var num = int.MinValue; + int? num = null; if (match.Groups["Num"].Success) { - if (int.TryParse(match.Groups["Num"].Value, out num)) + if (int.TryParse(match.Groups["Num"].Value, out var n)) { + num = n; if (match.Groups["Last"].Success) { // Make number negative diff --git a/Ical.Net/Serialization/DataTypes/WeekDaySerializer.cs b/Ical.Net/Serialization/DataTypes/WeekDaySerializer.cs index b4e4cd8e9..9308d6709 100644 --- a/Ical.Net/Serialization/DataTypes/WeekDaySerializer.cs +++ b/Ical.Net/Serialization/DataTypes/WeekDaySerializer.cs @@ -26,7 +26,7 @@ public override string SerializeToString(object obj) } var value = string.Empty; - if (ds.Offset != int.MinValue) + if (ds.Offset.HasValue) { value += ds.Offset; } @@ -64,4 +64,4 @@ public override object Deserialize(TextReader tr) ds.DayOfWeek = RecurrencePatternSerializer.GetDayOfWeek(match.Groups[3].Value); return ds; } -} \ No newline at end of file +} From 6adedcbae1d833317fc71a20bf4b9bc767b62b3f Mon Sep 17 00:00:00 2001 From: Markus Minichmayr Date: Mon, 9 Dec 2024 20:06:04 +0100 Subject: [PATCH 4/5] Remove obsolete method `RecurrencePatternSerializer.CheckRange` --- .../DataTypes/RecurrencePatternSerializer.cs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/Ical.Net/Serialization/DataTypes/RecurrencePatternSerializer.cs b/Ical.Net/Serialization/DataTypes/RecurrencePatternSerializer.cs index 343b60dc3..6990382d3 100644 --- a/Ical.Net/Serialization/DataTypes/RecurrencePatternSerializer.cs +++ b/Ical.Net/Serialization/DataTypes/RecurrencePatternSerializer.cs @@ -58,15 +58,6 @@ public virtual void CheckRange(string name, IList values, int min, int max) } } - public virtual void CheckRange(string name, IList values, int min, int max) - { - var allowZero = (min == 0 || max == 0); - foreach (var value in values) - { - CheckRange(name, value, min, max, allowZero); - } - } - public virtual void CheckRange(string name, int value, int min, int max) { var allowZero = min == 0 || max == 0; From 657609606b4c98ab74273685abd4d4d0940d47fa Mon Sep 17 00:00:00 2001 From: Markus Minichmayr Date: Mon, 9 Dec 2024 20:06:37 +0100 Subject: [PATCH 5/5] Test: Add WeekDay-related tests. --- Ical.Net.Tests/DataTypeTest.cs | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/Ical.Net.Tests/DataTypeTest.cs b/Ical.Net.Tests/DataTypeTest.cs index c24e2f4b0..82da677d6 100644 --- a/Ical.Net.Tests/DataTypeTest.cs +++ b/Ical.Net.Tests/DataTypeTest.cs @@ -3,6 +3,7 @@ // Licensed under the MIT license. // +using System.Collections.Generic; using Ical.Net.DataTypes; using NUnit.Framework; @@ -23,4 +24,35 @@ public void AttachmentConstructorMustAcceptNull() Assert.DoesNotThrow(() => { var o = new Attachment((byte[]) null); }); Assert.DoesNotThrow(() => { var o = new Attachment((string) null); }); } -} \ No newline at end of file + + public static IEnumerable TestWeekDayEqualsTestCases => [ + new(new WeekDay("MO"), new WeekDay("1MO")), + new(new WeekDay("1TU"), new WeekDay("-1TU")), + new(new WeekDay("2WE"), new WeekDay("-2WE")), + new(new WeekDay("TH"), new WeekDay("FR")), + new(new WeekDay("-5FR"), new WeekDay("FR")), + new(new WeekDay("SA"), null), + ]; + + [Test, TestCaseSource(nameof(TestWeekDayEqualsTestCases)), Category("DataType")] + public void TestWeekDayEquals(WeekDay w1, WeekDay w2) + { + Assert.Multiple(() => + { + Assert.That(w1.Equals(w1), Is.True); + Assert.That(w1.Equals(w2), Is.False); + }); + } + + [Test, TestCaseSource(nameof(TestWeekDayEqualsTestCases)), Category("DataType")] + public void TestWeekDayCompareTo(WeekDay w1, WeekDay w2) + { + Assert.Multiple(() => + { + Assert.That(w1.CompareTo(w1), Is.EqualTo(0)); + + if (w2 != null) + Assert.That(w1.CompareTo(w2), Is.Not.EqualTo(0)); + }); + } +}