Skip to content

Commit

Permalink
Remove mutable state from Evaluators and some related, poorly tested …
Browse files Browse the repository at this point in the history
…functions (#655)

* TodoEvaluator, Todo: Simplify `DetermineStartingRecurrence` by removing the dependency on `PeriodListEvaluator`. Make related methods private or internal. Avoid using `IEvaluator.Periods` so it can be removed.

* Remove much of the mutable state of `IEvaluator`, i.e. `.Evaluation*Bounds` and `.Periods` properties, because it isn't needed anymore and removing it vastly reduces complexity.

* Remove `CalendarEvent.OccursOn()` and `.OccursAt()`, because they rely on internal state of `Evaluator` that should be removed. They are fully untested, so the intended functionality and usage pattern are rather unclear. Moreover it seems that the functionality can be implemented by the user quite easily and exactly to their requirement.

* Remove `Evaluator.Evaluation*Bounds` and `.Clear()` because they are not needed anymore.

* Remove state `Alarm.Occurrences` because they are not needed anymore.

* Remove unused member `CalendarEvent._mEvaluator`.

* Remove unused `Evaluator` constructors .

* Remove unused `TimeZoneEvaluator`.
  • Loading branch information
minichma authored Nov 30, 2024
1 parent e40d238 commit e77681c
Show file tree
Hide file tree
Showing 17 changed files with 54 additions and 353 deletions.
3 changes: 0 additions & 3 deletions Ical.Net.Tests/RecurrenceTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2892,9 +2892,6 @@ public void GetOccurrences1()
evt.RecurrenceRules[0].ByHour.Add(9);
evt.RecurrenceRules[0].ByHour.Add(12);

// Clear the evaluation so we can calculate recurrences again.
evt.ClearEvaluation();

occurrences = evt.GetOccurrences(previousDateAndTime, end);
Assert.That(occurrences, Has.Count.EqualTo(10));

Expand Down
13 changes: 1 addition & 12 deletions Ical.Net/Calendar.cs
Original file line number Diff line number Diff line change
Expand Up @@ -198,17 +198,6 @@ public VTimeZone AddTimeZone(VTimeZone tz)
}


/// <summary>
/// Clears recurrence evaluations for recurring components.
/// </summary>
public void ClearEvaluation()
{
foreach (var recurrable in RecurringItems)
{
recurrable.ClearEvaluation();
}
}

/// <summary>
/// Returns a list of occurrences of each recurring component
/// for the date provided (<paramref name="dt"/>).
Expand Down Expand Up @@ -405,4 +394,4 @@ public VTimeZone AddLocalTimeZone(DateTime earliestDateTimeToSupport, bool inclu
this.AddChild(tz);
return tz;
}
}
}
10 changes: 1 addition & 9 deletions Ical.Net/CalendarCollection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,6 @@ public static CalendarCollection Load(TextReader tr)
return collection;
}

public void ClearEvaluation()
{
foreach (var iCal in this)
{
iCal.ClearEvaluation();
}
}

public HashSet<Occurrence> GetOccurrences(IDateTime dt)
{
var occurrences = new HashSet<Occurrence>();
Expand Down Expand Up @@ -158,4 +150,4 @@ public override bool Equals(object obj)
if (ReferenceEquals(this, obj)) return true;
return obj.GetType() == GetType() && Equals((CalendarEvent) obj);
}
}
}
27 changes: 12 additions & 15 deletions Ical.Net/CalendarComponents/Alarm.cs
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,9 @@ public virtual Trigger Trigger
set => Properties.Set(TriggerRelation.Key, value);
}

protected virtual IList<AlarmOccurrence> Occurrences { get; set; }

public Alarm()
{
Name = Components.Alarm;
Occurrences = new List<AlarmOccurrence>();
}

/// <summary>
Expand All @@ -77,13 +74,13 @@ public Alarm()
/// </summary>
public virtual IList<AlarmOccurrence> GetOccurrences(IRecurringComponent rc, IDateTime fromDate, IDateTime toDate)
{
Occurrences.Clear();

if (Trigger == null)
{
return Occurrences;
return [];
}

var occurrences = new List<AlarmOccurrence>();

// If the trigger is relative, it can recur right along with
// the recurring items, otherwise, it happens once and
// only once (at a precise time).
Expand Down Expand Up @@ -121,21 +118,21 @@ public virtual IList<AlarmOccurrence> GetOccurrences(IRecurringComponent rc, IDa
}
}

Occurrences.Add(new AlarmOccurrence(this, dt.Add(Trigger.Duration.Value), rc));
occurrences.Add(new AlarmOccurrence(this, dt.Add(Trigger.Duration.Value), rc));
}
}
else
{
var dt = Trigger.DateTime.Copy<IDateTime>();
dt.AssociatedObject = this;
Occurrences.Add(new AlarmOccurrence(this, dt, rc));
occurrences.Add(new AlarmOccurrence(this, dt, rc));
}

// If a REPEAT and DURATION value were specified,
// then handle those repetitions here.
AddRepeatedItems();
AddRepeatedItems(occurrences);

return Occurrences;
return occurrences;
}

/// <summary>
Expand Down Expand Up @@ -165,19 +162,19 @@ public virtual IList<AlarmOccurrence> Poll(IDateTime start, IDateTime end)
/// <c>DURATION</c> properties. Each recurrence of the alarm will
/// have its own set of generated repetitions.
/// </summary>
protected virtual void AddRepeatedItems()
private void AddRepeatedItems(List<AlarmOccurrence> occurrences)
{
var len = Occurrences.Count;
var len = occurrences.Count;
for (var i = 0; i < len; i++)
{
var ao = Occurrences[i];
var ao = occurrences[i];
var alarmTime = ao.DateTime.Copy<IDateTime>();

for (var j = 0; j < Repeat; j++)
{
alarmTime = alarmTime.Add(Duration);
Occurrences.Add(new AlarmOccurrence(this, alarmTime.Copy<IDateTime>(), ao.Component));
occurrences.Add(new AlarmOccurrence(this, alarmTime.Copy<IDateTime>(), ao.Component));
}
}
}
}
}
31 changes: 1 addition & 30 deletions Ical.Net/CalendarComponents/CalendarEvent.cs
Original file line number Diff line number Diff line change
Expand Up @@ -224,8 +224,6 @@ public string Transparency
set => Properties.Set(TransparencyType.Key, value);
}

private EventEvaluator _mEvaluator;

/// <summary>
/// Constructs an Event object, with an iCalObject
/// (usually an iCalendar object) as its parent.
Expand All @@ -239,36 +237,9 @@ private void Initialize()
{
Name = EventStatus.Name;

_mEvaluator = new EventEvaluator(this);
SetService(_mEvaluator);
SetService(new EventEvaluator(this));
}

/// <summary>
/// Use this method to determine if an event occurs on a given date.
/// <note type="caution">
/// This event should be called only after the Evaluate
/// method has calculated the dates for which this event occurs.
/// </note>
/// </summary>
/// <param name="dateTime">The date to test.</param>
/// <returns>True if the event occurs on the <paramref name="dateTime"/> provided, False otherwise.</returns>
public virtual bool OccursOn(IDateTime dateTime)
{
return _mEvaluator.Periods.Any(p => p.StartTime.Date == dateTime.Date || // It's the start date OR
(p.StartTime.Date <= dateTime.Date && // It's after the start date AND
(p.EndTime.HasTime && p.EndTime.Date >= dateTime.Date || // an end time was specified, and it's after the test date
(!p.EndTime.HasTime && p.EndTime.Date > dateTime.Date))));
}

/// <summary>
/// Use this method to determine if an event begins at a given date and time.
/// </summary>
/// <param name="dateTime">The date and time to test.</param>
/// <returns>True if the event begins at the given date and time</returns>
public virtual bool OccursAt(IDateTime dateTime)
{
return _mEvaluator.Periods.Any(p => p.StartTime.Equals(dateTime));
}

/// <summary>
/// Determines whether or not the <see cref="CalendarEvent"/> is actively displayed
Expand Down
4 changes: 1 addition & 3 deletions Ical.Net/CalendarComponents/RecurringComponent.cs
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,6 @@ protected override void OnDeserializing(StreamingContext context)
Initialize();
}

public virtual void ClearEvaluation() => RecurrenceUtil.ClearEvaluation(this);

public virtual HashSet<Occurrence> GetOccurrences(IDateTime dt) => RecurrenceUtil.GetOccurrences(this, dt, EvaluationIncludesReferenceDate);

public virtual HashSet<Occurrence> GetOccurrences(DateTime dt)
Expand Down Expand Up @@ -243,4 +241,4 @@ public override int GetHashCode()
return hashCode;
}
}
}
}
6 changes: 3 additions & 3 deletions Ical.Net/CalendarComponents/Todo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -160,9 +160,9 @@ public virtual bool IsCompleted(IDateTime currDt)
}

// Evaluate to the previous occurrence.
_mEvaluator.EvaluateToPreviousOccurrence(Completed, currDt);
var periods = _mEvaluator.EvaluateToPreviousOccurrence(Completed, currDt);

return _mEvaluator.Periods.Cast<Period>().All(p => !p.StartTime.GreaterThan(Completed) || !currDt.GreaterThanOrEqual(p.StartTime));
return periods.Cast<Period>().All(p => !p.StartTime.GreaterThan(Completed) || !currDt.GreaterThanOrEqual(p.StartTime));
}
return false;
}
Expand Down Expand Up @@ -212,4 +212,4 @@ private void ExtrapolateTimes(int source)
DtStart = Due.Subtract(Duration);
}
}
}
}
46 changes: 2 additions & 44 deletions Ical.Net/Evaluation/Evaluator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,37 +13,16 @@ namespace Ical.Net.Evaluation;

public abstract class Evaluator : IEvaluator
{
private DateTime _mEvaluationStartBounds = DateTime.MaxValue;
private DateTime _mEvaluationEndBounds = DateTime.MinValue;

private ICalendarObject _mAssociatedObject;
private readonly ICalendarDataType _mAssociatedDataType;

protected HashSet<Period> MPeriods;

protected Evaluator()
{
Initialize();
}

protected Evaluator(ICalendarObject associatedObject)
{
_mAssociatedObject = associatedObject;

Initialize();
}

protected Evaluator(ICalendarDataType dataType)
{
_mAssociatedDataType = dataType;

Initialize();
}

private void Initialize()
{
Calendar = CultureInfo.CurrentCulture.Calendar;
MPeriods = new HashSet<Period>();
}

protected IDateTime ConvertToIDateTime(DateTime dt, IDateTime referenceDate)
Expand Down Expand Up @@ -90,32 +69,11 @@ protected void IncrementDate(ref DateTime dt, RecurrencePattern pattern, int int

public System.Globalization.Calendar Calendar { get; private set; }

public virtual DateTime EvaluationStartBounds
{
get => _mEvaluationStartBounds;
set => _mEvaluationStartBounds = value;
}

public virtual DateTime EvaluationEndBounds
{
get => _mEvaluationEndBounds;
set => _mEvaluationEndBounds = value;
}

public virtual ICalendarObject AssociatedObject
{
get => _mAssociatedObject ?? _mAssociatedDataType?.AssociatedObject;
get => _mAssociatedObject;
protected set => _mAssociatedObject = value;
}

public virtual HashSet<Period> Periods => MPeriods;

public virtual void Clear()
{
_mEvaluationStartBounds = DateTime.MaxValue;
_mEvaluationEndBounds = DateTime.MinValue;
MPeriods.Clear();
}

public abstract HashSet<Period> Evaluate(IDateTime referenceDate, DateTime periodStart, DateTime periodEnd, bool includeReferenceDateInResults);
}
}
10 changes: 5 additions & 5 deletions Ical.Net/Evaluation/EventEvaluator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ public EventEvaluator(CalendarEvent evt) : base(evt) { }
public override HashSet<Period> Evaluate(IDateTime referenceTime, DateTime periodStart, DateTime periodEnd, bool includeReferenceDateInResults)
{
// Evaluate recurrences normally
base.Evaluate(referenceTime, periodStart, periodEnd, includeReferenceDateInResults);
var periods = base.Evaluate(referenceTime, periodStart, periodEnd, includeReferenceDateInResults);

foreach (var period in Periods)
foreach (var period in periods)
{
period.Duration = CalendarEvent.GetFirstDuration();
period.EndTime = period.Duration == default
Expand All @@ -52,14 +52,14 @@ public override HashSet<Period> Evaluate(IDateTime referenceTime, DateTime perio
}

// Ensure each period has a duration
foreach (var period in Periods.Where(p => p.EndTime == null))
foreach (var period in periods.Where(p => p.EndTime == null))
{
period.Duration = CalendarEvent.GetFirstDuration();
period.EndTime = period.Duration == default
? period.StartTime
: period.StartTime.Add(CalendarEvent.GetFirstDuration());
}

return Periods;
return periods;
}
}
}
31 changes: 1 addition & 30 deletions Ical.Net/Evaluation/IEvaluator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,40 +16,11 @@ public interface IEvaluator
/// </summary>
System.Globalization.Calendar Calendar { get; }

/// <summary>
/// The start bounds of the evaluation. This gives
/// the first date/time that is covered by the evaluation.
/// This together with EvaluationEndBounds determines
/// what time frames have already been evaluated, so
/// duplicate evaluation doesn't occur.
/// </summary>
DateTime EvaluationStartBounds { get; }

/// <summary>
/// The end bounds of the evaluation.
/// See <see cref="EvaluationStartBounds"/> for more info.
/// </summary>
DateTime EvaluationEndBounds { get; }

/// <summary>
/// Gets a list of periods collected so far during
/// the evaluation process.
/// </summary>
HashSet<Period> Periods { get; }

/// <summary>
/// Gets the object associated with this evaluator.
/// </summary>
ICalendarObject AssociatedObject { get; }

/// <summary>
/// Clears the evaluation, eliminating all data that has
/// been collected up to this point. Since this data is cached
/// as needed, this method can be useful to gather information
/// that is guaranteed to not be out-of-date.
/// </summary>
void Clear();

/// <summary>
/// Evaluates this item to determine the dates and times for which it occurs/recurs.
/// This method only evaluates items which occur/recur between <paramref name="periodStart"/>
Expand All @@ -74,4 +45,4 @@ public interface IEvaluator
/// each date/time when this item occurs/recurs.
/// </returns>
HashSet<Period> Evaluate(IDateTime referenceDate, DateTime periodStart, DateTime periodEnd, bool includeReferenceDateInResults);
}
}
5 changes: 1 addition & 4 deletions Ical.Net/Evaluation/RecurrencePatternEvaluator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -963,14 +963,11 @@ public override HashSet<Period> Evaluate(IDateTime referenceDate, DateTime perio

// Enforce evaluation restrictions on the pattern.
EnforceEvaluationRestrictions(pattern);
Periods.Clear();

var periodQuery = GetDates(referenceDate, periodStart, periodEnd, -1, pattern, includeReferenceDateInResults)
.Select(dt => CreatePeriod(dt, referenceDate));

Periods.UnionWith(periodQuery);

return Periods;
return new HashSet<Period>(periodQuery);
}

private static IDateTime MatchTimeZone(IDateTime dt1, IDateTime dt2)
Expand Down
Loading

0 comments on commit e77681c

Please sign in to comment.