Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from
Dec 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 33 additions & 1 deletion Ical.Net.Tests/DataTypeTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// Licensed under the MIT license.
//

using System.Collections.Generic;
using Ical.Net.DataTypes;
using NUnit.Framework;

Expand All @@ -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));
});
}
}
3 changes: 1 addition & 2 deletions Ical.Net/Constants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,6 @@ public enum FrequencyType
/// </summary>
public enum FrequencyOccurrence
{
None = int.MinValue,
Last = -1,
SecondToLast = -2,
ThirdToLast = -3,
Expand Down Expand Up @@ -374,4 +373,4 @@ public static class RegexDefaults
/// The default timeout for regular expressions.
/// </summary>
public static readonly TimeSpan Timeout = TimeSpan.FromMilliseconds(TimeoutMilliseconds);
}
}
10 changes: 4 additions & 6 deletions Ical.Net/DataTypes/RecurrencePattern.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand All @@ -39,9 +39,7 @@ public class RecurrencePattern : EncodableDataType
/// </summary>
public int Interval
{
get => _interval == int.MinValue
? 1
: _interval;
get => _interval ?? 1;
set => _interval = value;
}

Expand Down Expand Up @@ -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);
Expand Down
13 changes: 6 additions & 7 deletions Ical.Net/DataTypes/WeekDay.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
//

using System;
using System.Collections.Generic;
using System.IO;
using Ical.Net.Serialization.DataTypes;

Expand All @@ -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()
{
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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;
}
}
}
18 changes: 11 additions & 7 deletions Ical.Net/Evaluation/RecurrencePatternEvaluator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -221,14 +221,18 @@

// 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");

Check warning on line 235 in Ical.Net/Evaluation/RecurrencePatternEvaluator.cs

View check run for this annotation

Codecov / codecov/patch

Ical.Net/Evaluation/RecurrencePatternEvaluator.cs#L235

Added line #L235 was not covered by tests
}

// Do the enumeration in a separate method, as it is a generator method that is
Expand Down Expand Up @@ -256,7 +260,7 @@
break;
}

if (pattern.Count >= 1 && dateCount >= pattern.Count)
if (dateCount >= pattern.Count)
{
break;
}
Expand All @@ -281,7 +285,7 @@
// 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;
}
Expand Down Expand Up @@ -747,9 +751,9 @@
/// </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;
}
Expand All @@ -758,11 +762,11 @@
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;
}
Expand Down
2 changes: 1 addition & 1 deletion Ical.Net/Evaluation/TodoEvaluator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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>();
}
Expand Down
29 changes: 20 additions & 9 deletions Ical.Net/Serialization/DataTypes/RecurrencePatternSerializer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,24 @@
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)") + ".");

Check warning on line 78 in Ical.Net/Serialization/DataTypes/RecurrencePatternSerializer.cs

View check run for this annotation

Codecov / codecov/patch

Ical.Net/Serialization/DataTypes/RecurrencePatternSerializer.cs#L77-L78

Added lines #L77 - L78 were not covered by tests
}
}

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)") + ".");
Expand Down Expand Up @@ -133,11 +148,6 @@
//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);
Expand All @@ -160,7 +170,7 @@
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);
}
Expand Down Expand Up @@ -410,11 +420,12 @@
}
else if ((match = RelativeDaysOfWeek.Match(item)).Success)
{
var num = int.MinValue;
int? num = null;

Check warning on line 423 in Ical.Net/Serialization/DataTypes/RecurrencePatternSerializer.cs

View check run for this annotation

Codecov / codecov/patch

Ical.Net/Serialization/DataTypes/RecurrencePatternSerializer.cs#L423

Added line #L423 was not covered by tests
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;

Check warning on line 428 in Ical.Net/Serialization/DataTypes/RecurrencePatternSerializer.cs

View check run for this annotation

Codecov / codecov/patch

Ical.Net/Serialization/DataTypes/RecurrencePatternSerializer.cs#L428

Added line #L428 was not covered by tests
if (match.Groups["Last"].Success)
{
// Make number negative
Expand Down
4 changes: 2 additions & 2 deletions Ical.Net/Serialization/DataTypes/WeekDaySerializer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -64,4 +64,4 @@ public override object Deserialize(TextReader tr)
ds.DayOfWeek = RecurrencePatternSerializer.GetDayOfWeek(match.Groups[3].Value);
return ds;
}
}
}
Loading