Skip to content

Commit

Permalink
Change Period.Duration from TimeSpan to Duration. Remove extrap…
Browse files Browse the repository at this point in the history 
…olation and introduce `GetEffectiveDuration()`, `GetEffectiveEndTime()`.
  • Loading branch information
@minichma
minichma committed Dec 26, 2024
1 parent c58282a commit 19e98d7
Show file tree
Hide file tree
Showing 6 changed files with 62 additions and 79 deletions.
2 changes: 1 addition & 1 deletion Ical.Net.Tests/CopyComponentTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ public void CopyFreeBusyTest()
{
Start = new CalDateTime(_now),
End = new CalDateTime(_later),
Entries = { new FreeBusyEntry { Language = "English", StartTime = new CalDateTime(2024, 10, 1), Duration = TimeSpan.FromDays(1), Status = FreeBusyStatus.Busy } }
Entries = { new FreeBusyEntry { Language = "English", StartTime = new CalDateTime(2024, 10, 1), Duration = Duration.FromDays(1), Status = FreeBusyStatus.Busy } }
};

var copy = orig.Copy<FreeBusy>();
Expand Down
32 changes: 16 additions & 16 deletions Ical.Net.Tests/PeriodTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public void CreatePeriodWithArguments()
{
var period = new Period(new CalDateTime(2025, 1, 1, 0, 0, 0, "America/New_York"));
var periodWithEndTime = new Period(new CalDateTime(2025, 1, 1, 0, 0, 0, "America/New_York"), new CalDateTime(2025, 1, 1, 1, 0, 0, "America/New_York"));
var periodWithDuration = new Period(new CalDateTime(2025, 1, 1, 0, 0, 0, "America/New_York"), new TimeSpan(1, 0, 0));
var periodWithDuration = new Period(new CalDateTime(2025, 1, 1, 0, 0, 0, "America/New_York"), Duration.FromHours(1));

Assert.Multiple(() =>
{
Expand All @@ -27,12 +27,12 @@ public void CreatePeriodWithArguments()
Assert.That(period.Duration, Is.Null);

Assert.That(periodWithEndTime.StartTime, Is.EqualTo(new CalDateTime(2025, 1, 1, 0, 0, 0, "America/New_York")));
Assert.That(periodWithEndTime.EndTime, Is.EqualTo(new CalDateTime(2025, 1, 1, 1, 0, 0, "America/New_York")));
Assert.That(periodWithEndTime.Duration, Is.EqualTo(new TimeSpan(1, 0, 0)));
Assert.That(periodWithEndTime.GetEffectiveEndTime(), Is.EqualTo(new CalDateTime(2025, 1, 1, 1, 0, 0, "America/New_York")));
Assert.That(periodWithEndTime.GetEffectiveDuration(), Is.EqualTo(Duration.FromHours(1)));

Assert.That(periodWithDuration.StartTime, Is.EqualTo(new CalDateTime(2025, 1, 1, 0, 0, 0, "America/New_York")));
Assert.That(periodWithDuration.EndTime, Is.EqualTo(new CalDateTime(2025, 1, 1, 1, 0, 0, "America/New_York")));
Assert.That(periodWithDuration.Duration, Is.EqualTo(new TimeSpan(1, 0, 0)));
Assert.That(periodWithDuration.GetEffectiveEndTime(), Is.EqualTo(new CalDateTime(2025, 1, 1, 1, 0, 0, "America/New_York")));
Assert.That(periodWithDuration.GetEffectiveDuration(), Is.EqualTo(Duration.FromHours(1)));
});
}

Expand All @@ -42,7 +42,7 @@ public void CreatePeriodWithInvalidArgumentsShouldThrow()
Assert.Throws<ArgumentException>(() => _ = new Period(new CalDateTime(2025, 1, 1, 0, 0, 0, "America/New_York"),
new CalDateTime(2025, 1, 1, 0, 0, 0, "America/New_York")));
Assert.Throws<ArgumentException>(() =>
_ = new Period(new CalDateTime(2025, 1, 1, 0, 0, 0, "America/New_York"), new TimeSpan(-1, 0, 0)));
_ = new Period(new CalDateTime(2025, 1, 1, 0, 0, 0, "America/New_York"), Duration.FromHours(-1)));
}
[Test]
public void SetAndGetStartTime()
Expand All @@ -61,31 +61,31 @@ public void SetEndTime_GetDuration()
var endTime = new CalDateTime(2025, 1, 31, 0, 0, 0);
period.EndTime = endTime;

Assert.That(period.GetEffectiveEndTime(), Is.EqualTo(endTime));
Assert.That(period.EndTime, Is.EqualTo(endTime));
Assert.That(period.GetOriginalValues().EndTime, Is.EqualTo(endTime));
Assert.That(period.GetOriginalValues().Duration, Is.Null);
Assert.That(period.Duration, Is.EqualTo(new TimeSpan(30, 0, 0, 0)));
Assert.That(period.Duration, Is.Null);
Assert.That(period.GetEffectiveDuration(), Is.EqualTo(Duration.FromDays(30)));
}

[Test]
public void SetDuration_GetEndTime()
{
var period = new Period(new CalDateTime(2025, 1, 1, 0, 0, 0));
var duration = new TimeSpan(1, 0, 0);
var duration = Duration.FromHours(1);
period.Duration = duration;

Assert.That(period.GetEffectiveDuration(), Is.EqualTo(duration));
Assert.That(period.Duration, Is.EqualTo(duration));
Assert.That(period.GetOriginalValues().Duration, Is.EqualTo(duration));
Assert.That(period.GetOriginalValues().EndTime, Is.Null);
Assert.That(period.EndTime, Is.EqualTo(new CalDateTime(2025, 1, 1, 1, 0, 0)));
Assert.That(period.EndTime, Is.Null);
Assert.That(period.GetEffectiveEndTime(), Is.EqualTo(new CalDateTime(2025, 1, 1, 1, 0, 0)));
}

[Test]
public void CollidesWithPeriod()
{
var period1 = new Period(new CalDateTime(2025, 1, 1, 0, 0, 0), new TimeSpan(1, 0, 0));
var period2 = new Period(new CalDateTime(2025, 1, 1, 0, 30, 0), new TimeSpan(1, 0, 0));
var period3 = new Period(new CalDateTime(2025, 1, 1, 1, 30, 0), new TimeSpan(1, 0, 0));
var period1 = new Period(new CalDateTime(2025, 1, 1, 0, 0, 0), Duration.FromHours(1));
var period2 = new Period(new CalDateTime(2025, 1, 1, 0, 30, 0), Duration.FromHours(1));
var period3 = new Period(new CalDateTime(2025, 1, 1, 1, 30, 0), Duration.FromHours(1));

Assert.Multiple(() =>
{
Expand Down
9 changes: 4 additions & 5 deletions Ical.Net/CalendarComponents/Alarm.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public virtual IList<AlarmOccurrence> GetOccurrences(IRecurringComponent rc, IDa
fromDate = rc.Start.Copy<IDateTime>();
}

var d = default(TimeSpan);
Duration? d = null;

Check warning on line 96 in Ical.Net/CalendarComponents/Alarm.cs

View check run for this annotation

Codecov / codecov/patch

Ical.Net/CalendarComponents/Alarm.cs#L96 

Added line #L96 was not covered by tests
foreach (var o in rc.GetOccurrences(fromDate, toDate))
{
var dt = o.Period.StartTime;
Expand All @@ -102,16 +102,15 @@ public virtual IList<AlarmOccurrence> GetOccurrences(IRecurringComponent rc, IDa
if (o.Period.EndTime != null)
{
dt = o.Period.EndTime;
if (d == default)
if (d == null)
{
d = o.Period.Duration!.Value; // the getter always returns a value
}
}
// Use the "last-found" duration as a reference point
else if (d != default(TimeSpan))
else if (d != null)
{
// TODO: Nominal or exact?
dt = o.Period.StartTime.Add(d.ToDuration());
dt = o.Period.StartTime.Add(d.Value);

Check warning on line 113 in Ical.Net/CalendarComponents/Alarm.cs

View check run for this annotation

Codecov / codecov/patch

Ical.Net/CalendarComponents/Alarm.cs#L113 

Added line #L113 was not covered by tests
}
else
{
Expand Down
93 changes: 38 additions & 55 deletions Ical.Net/DataTypes/Period.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public Period(IDateTime start, IDateTime? end = null)
}

StartTime = start ?? throw new ArgumentNullException(nameof(start));
_endTime = end;
EndTime = end;
}

/// <summary>
Expand All @@ -46,15 +46,15 @@ public Period(IDateTime start, IDateTime? end = null)
/// <param name="start"></param>
/// <param name="duration"></param>
/// <exception cref="ArgumentException"></exception>
public Period(IDateTime start, TimeSpan duration)
public Period(IDateTime start, Duration duration)
{
if (duration < TimeSpan.Zero)
if (duration.IsNegative)
{
throw new ArgumentException("Duration must be greater than or equal to zero.", nameof(duration));
}

StartTime = start;
_duration = duration;
Duration = duration;
}

/// <inheritdoc/>
Expand All @@ -65,8 +65,8 @@ public override void CopyFrom(ICopyable obj)
if (obj is not Period p) return;

StartTime = p.StartTime.Copy<IDateTime>();
_endTime = p._endTime?.Copy<IDateTime>();
_duration = p._duration;
EndTime = p.EndTime?.Copy<IDateTime>();
Duration = p.Duration;
}

protected bool Equals(Period other) => Equals(StartTime, other.StartTime) && Equals(EndTime, other.EndTime) && Duration.Equals(other.Duration);
Expand Down Expand Up @@ -103,8 +103,6 @@ public override string ToString()
/// </summary>
public virtual IDateTime StartTime { get; set; } = null!;

private IDateTime? _endTime;

/// <summary>
/// Gets either the end time of the period that was set,
/// or calculates the exact end time based on the nominal duration.
Expand All @@ -113,20 +111,7 @@ public override string ToString()
/// Either the <see cref="EndTime"/> or the <see cref="Duration"/> can be set at a time.
/// The last one set will be stored, and the other will be calculated.
/// </summary>
public virtual IDateTime? EndTime
{
get => _endTime ?? (_duration != null ? StartTime.Add(_duration.Value.ToDuration()) : null);
set
{
_endTime = value;
if (_endTime != null)
{
_duration = null;
}
}
}

private TimeSpan? _duration;
public virtual IDateTime? EndTime { get; set; }

/// <summary>
/// Gets either the nominal duration of the period that was set,
Expand All @@ -139,34 +124,38 @@ public virtual IDateTime? EndTime
/// A <see cref="Period"/> that has a date-only <see cref="StartTime"/>, no <see cref="EndTime"/>
/// and no duration set, is considered to last for one day.
/// </summary>
public virtual TimeSpan? Duration
{
get
{
if (_endTime == null && !StartTime.HasTime)
{
return TimeSpan.FromDays(1);
}
return _duration ?? _endTime?.Subtract(StartTime).ToTimeSpan();
}
set
{
_duration = value;
if (_duration != null)
{
_endTime = null;
}
}
}
public virtual Duration? Duration { get; set; }

/// <summary>
/// Gets the original start time, end time, and duration as they were set,
/// while <see cref="EndTime"/> or <see cref="Duration"/> properties may be calculated.
/// </summary>
/// <returns></returns>
public (IDateTime StartTime, IDateTime? EndTime, TimeSpan? Duration) GetOriginalValues()
public (IDateTime StartTime, IDateTime? EndTime, Duration? Duration) GetOriginalValues()
{
return (StartTime, EndTime, Duration);

Check warning on line 136 in Ical.Net/DataTypes/Period.cs

View check run for this annotation

Codecov / codecov/patch

Ical.Net/DataTypes/Period.cs#L136 

Added line #L136 was not covered by tests
}

internal Duration GetEffectiveDuration()
{
return (StartTime, _endTime, _duration);
if (Duration is { } d)
return d;

if (EndTime is { } endTime)
return endTime.Subtract(StartTime);

if (!StartTime.HasTime)
return DataTypes.Duration.FromDays(1);

Check warning on line 148 in Ical.Net/DataTypes/Period.cs

View check run for this annotation

Codecov / codecov/patch

Ical.Net/DataTypes/Period.cs#L148 

Added line #L148 was not covered by tests

return DataTypes.Duration.Zero;

Check warning on line 150 in Ical.Net/DataTypes/Period.cs

View check run for this annotation

Codecov / codecov/patch

Ical.Net/DataTypes/Period.cs#L150 

Added line #L150 was not covered by tests
}

internal IDateTime GetEffectiveEndTime()
{
if (EndTime is { } endTime)
return endTime;

return StartTime.Add(GetEffectiveDuration());
}

/// <summary>
Expand All @@ -184,8 +173,9 @@ public virtual bool Contains(IDateTime? dt)
return false;
}

var endTime = GetEffectiveEndTime();
// End time is exclusive
return EndTime == null || EndTime.GreaterThan(dt);
return endTime == null || endTime.GreaterThan(dt);
}

/// <summary>
Expand All @@ -199,17 +189,10 @@ public virtual bool Contains(IDateTime? dt)
/// <param name="period"></param>
/// <returns></returns>
public virtual bool CollidesWith(Period period)
{
// Check if the start or end time of the given period is within the current period
var startContained = Contains(period.StartTime);
var endContained = period.EndTime != null && Contains(period.EndTime);

// Check if the current period is completely within the given period
var currentStartContained = period.Contains(StartTime);
var currentEndContained = EndTime != null && period.Contains(EndTime);

return startContained || endContained || currentStartContained || currentEndContained;
}
=> Contains(period.StartTime)
|| period.Contains(StartTime)
|| Contains(period.GetEffectiveEndTime())
|| period.Contains(GetEffectiveEndTime());

public int CompareTo(Period? other)
{
Expand Down
3 changes: 2 additions & 1 deletion Ical.Net/Evaluation/EventEvaluator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,8 @@ and it may differ from the time span added to the period start time.
}

// Return the Period object with the calculated end time and duration.
period.Duration = endTime.SubtractExact(period.StartTime); // exact duration
// TODO: Should we preserve both, endTime and duration?
period.Duration = endTime.Subtract(period.StartTime); // exact duration
period.EndTime = endTime; // Only EndTime is relevant for further processing.

return period;
Expand Down
2 changes: 1 addition & 1 deletion Ical.Net/Serialization/DataTypes/PeriodSerializer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ public override object Deserialize(TextReader tr)
p.EndTime = dtSerializer.Deserialize(new StringReader(values[1])) as IDateTime;
if (p.EndTime == null)
{
p.Duration = ((Duration)durationSerializer.Deserialize(new StringReader(values[1]))).ToTimeSpan();
p.Duration = (Duration)durationSerializer.Deserialize(new StringReader(values[1]));
}

// Only return an object if it has been deserialized correctly.
Expand Down

0 comments on commit 19e98d7

Please sign in to comment.