Make RecurrencePattern.Count, .Interval and WeekDay.Offset nullable to avoid using MinValue as magic number. (#667)

* Make `RucrrencePattern.Count` nullable to avoid using magic number `int.MinValue`.

* Make `RucrrencePattern.Interval` nullable to avoid using magic number `int.MinValue`.

* Make `WeekDay.Offset` nullable to avoid using `int.MinValue` as magic number.

* Remove obsolete method `RecurrencePatternSerializer.CheckRange`

* Test: Add WeekDay-related tests.

Author: minichma

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); });
     }
-}
+
+    public static IEnumerable<TestCaseData> 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));
+        });
+    }
+}

Ical.Net/Constants.cs

@@ -211,7 +211,6 @@ public enum FrequencyType
 /// </summary>
 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.
     /// </summary>
     public static readonly TimeSpan Timeout = TimeSpan.FromMilliseconds(TimeoutMilliseconds);
-}
+}

Ical.Net/DataTypes/RecurrencePattern.cs

@@ -20,7 +20,7 @@ namespace Ical.Net.DataTypes;
 /// </summary>
 public class RecurrencePattern : EncodableDataType
 {
-    private int _interval = int.MinValue;
+    private int? _interval;
 #pragma warning disable 0618
     private RecurrenceRestrictionType? _restrictionType;
     private RecurrenceEvaluationModeType? _evaluationMode;
@@ -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; }
 
     /// <summary>
     /// Specifies how often the recurrence should repeat.
@@ -39,9 +39,7 @@ public class RecurrencePattern : EncodableDataType
     /// </summary>
     public int Interval
     {
-        get => _interval == int.MinValue
-            ? 1
-            : _interval;
+        get => _interval ?? 1;
         set => _interval = value;
     }
 
@@ -181,7 +179,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);

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;
 /// </summary>
 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();
 
     /// <inheritdoc/>
     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<int?>.Default.Compare(Offset, weekday.Offset);
         }
         return compare;
     }
-}
+}

Ical.Net/Evaluation/RecurrencePatternEvaluator.cs

@@ -221,14 +221,18 @@ private IEnumerable<DateTime> 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)
             {
                 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<DateTime> EnumerateDates(DateTime originalDate, DateTime see
                 break;
             }
 
-            if (pattern.Count >= 1 && dateCount >= pattern.Count)
+            if (dateCount >= pattern.Count)
             {
                 break;
             }
@@ -281,7 +285,7 @@ private IEnumerable<DateTime> 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;
                     }
@@ -747,9 +751,9 @@ private static int GetWeekDayOffset(DateTime date, DayOfWeek startOfWeek)
     /// </summary>
     /// <param name="dates">The list from which to extract the element.</param>
     /// <param name="offset">The position of the element to extract.</param>
-    private List<DateTime> GetOffsetDates(List<DateTime> dates, int offset)
+    private List<DateTime> GetOffsetDates(List<DateTime> dates, int? offset)
     {
-        if (offset == int.MinValue)
+        if (offset is null)
         {
             return dates;
         }
@@ -758,11 +762,11 @@ private List<DateTime> GetOffsetDates(List<DateTime> 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;
     }

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<IDateTime>();
         }

Ical.Net/Serialization/DataTypes/RecurrencePatternSerializer.cs

@@ -64,9 +64,24 @@ public virtual void CheckRange(string name, int value, int min, int max)
         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)))
+        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)") + ".");
+        }
+    }
+
+    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)") + ".");
@@ -133,11 +148,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);
@@ -160,7 +170,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);
         }
@@ -410,11 +420,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

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;
     }
-}
+}