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

v3.6.0 #61

Merged
merged 14 commits into from
Nov 5, 2024
Merged
Show file tree
Hide file tree
Changes from 4 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
2 changes: 1 addition & 1 deletion Nhl.Api.Common/Http/NhlApiWebHttpClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
public class NhlApiWebHttpClient : NhlApiHttpClient
{
private static readonly object _lock = new object();
private static HttpClient _httpClient;

Check warning on line 12 in Nhl.Api.Common/Http/NhlApiWebHttpClient.cs

View workflow job for this annotation

GitHub Actions / build

Non-nullable field '_httpClient' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the field as nullable.

/// <summary>
/// The NHL web api for the NHL HTTP API Web NHL-e endpoint
Expand All @@ -20,7 +20,7 @@
/// <summary>
/// The dedicated NHL web api HTTP Client for the Nhl.Api
/// </summary>
public NhlApiWebHttpClient() : base(clientApiUri: ClientApiUrl, clientVersion: "v1", timeoutInSeconds: 30)
public NhlApiWebHttpClient() : base(clientApiUri: ClientApiUrl, clientVersion: "v1", timeoutInSeconds: 60)
{

}
Expand Down
4 changes: 2 additions & 2 deletions Nhl.Api.Common/Http/NhlCmsHttpClient.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;
using System.Net.Http;

namespace Nhl.Api.Common.Http;
Expand All @@ -8,7 +8,7 @@
public class NhlCmsHttpClient : NhlApiHttpClient
{
private static readonly object _lock = new object();
private static HttpClient _httpClient;

Check warning on line 11 in Nhl.Api.Common/Http/NhlCmsHttpClient.cs

View workflow job for this annotation

GitHub Actions / build

Non-nullable field '_httpClient' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the field as nullable.

/// <summary>
/// The dedicated NHL HTTP API endpoint for NHL player images and content
Expand All @@ -18,7 +18,7 @@
/// <summary>
/// The dedicated NHL HTTP client for NHL player images and content
/// </summary>
public NhlCmsHttpClient() : base(clientApiUri: ClientApiUrl, clientVersion: string.Empty, timeoutInSeconds: 30)
public NhlCmsHttpClient() : base(clientApiUri: ClientApiUrl, clientVersion: string.Empty, timeoutInSeconds: 60)
{
}

Expand Down
4 changes: 2 additions & 2 deletions Nhl.Api.Common/Http/NhlEApiHttpClient.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;
using System.Net.Http;

namespace Nhl.Api.Common.Http;
Expand All @@ -8,7 +8,7 @@
public class NhlEApiHttpClient : NhlApiHttpClient
{
private static readonly object _lock = new object();
private static HttpClient _httpClient;

Check warning on line 11 in Nhl.Api.Common/Http/NhlEApiHttpClient.cs

View workflow job for this annotation

GitHub Actions / build

Non-nullable field '_httpClient' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the field as nullable.

/// <summary>
/// The NHLe Api for the NHLe Api HTTP API Web NHL-e endpoint for statistics
Expand All @@ -19,7 +19,7 @@
/// <summary>
/// The dedicated NHLe Api web api HTTP Client for the Nhl.Api
/// </summary>
public NhlEApiHttpClient() : base(clientApiUri: ClientApiUrl, clientVersion: string.Empty, timeoutInSeconds: 30)
public NhlEApiHttpClient() : base(clientApiUri: ClientApiUrl, clientVersion: string.Empty, timeoutInSeconds: 60)
{

}
Expand Down
2 changes: 2 additions & 0 deletions Nhl.Api.Common/Http/NhlScoresHtmlReportsApiHttpClient.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
using System;
using System.Net.Http;
using System.Threading;
using System.Threading.Tasks;

namespace Nhl.Api.Common.Http;
/// <summary>
Expand All @@ -8,7 +10,7 @@
public class NhlScoresHtmlReportsApiHttpClient : NhlApiHttpClient
{
private static readonly object _lock = new object();
private static HttpClient _httpClient;

Check warning on line 13 in Nhl.Api.Common/Http/NhlScoresHtmlReportsApiHttpClient.cs

View workflow job for this annotation

GitHub Actions / build

Non-nullable field '_httpClient' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the field as nullable.

/// <summary>
/// The NHL endpoint for HTML reports
Expand Down
2 changes: 1 addition & 1 deletion Nhl.Api.Common/Http/NhlShiftChartHttpClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
public class NhlShiftChartHttpClient : NhlApiHttpClient
{
private static readonly object _lock = new object();
private static HttpClient _httpClient;

Check warning on line 11 in Nhl.Api.Common/Http/NhlShiftChartHttpClient.cs

View workflow job for this annotation

GitHub Actions / build

Non-nullable field '_httpClient' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the field as nullable.

/// <summary>
/// The dedicated NHL HTTP API endpoint for the shift charts for individual live game feeds
Expand All @@ -18,7 +18,7 @@
/// <summary>
/// The dedicated NHL HTTP client for the shift charts for individual live game feeds
/// </summary>
public NhlShiftChartHttpClient() : base(clientApiUri: ClientApiUrl, clientVersion: string.Empty, timeoutInSeconds: 30)
public NhlShiftChartHttpClient() : base(clientApiUri: ClientApiUrl, clientVersion: string.Empty, timeoutInSeconds: 60)
{
}

Expand Down
2 changes: 1 addition & 1 deletion Nhl.Api.Common/Http/NhlStaticAssetsApiHttpClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
public class NhlStaticAssetsApiHttpClient : NhlApiHttpClient
{
private static readonly object _lock = new object();
private static HttpClient _httpClient;

Check warning on line 11 in Nhl.Api.Common/Http/NhlStaticAssetsApiHttpClient.cs

View workflow job for this annotation

GitHub Actions / build

Non-nullable field '_httpClient' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the field as nullable.
/// <summary>
/// The dedicated NHL static assets NHL HTTP API endpoint
/// </summary>
Expand All @@ -17,7 +17,7 @@
/// <summary>
/// The dedicated NHL static assets HTTP Client for the Nhl.Api
/// </summary>
public NhlStaticAssetsApiHttpClient() : base(clientApiUri: ClientApiUrl, clientVersion: string.Empty, timeoutInSeconds: 30)
public NhlStaticAssetsApiHttpClient() : base(clientApiUri: ClientApiUrl, clientVersion: string.Empty, timeoutInSeconds: 60)
{
}

Expand Down
2 changes: 1 addition & 1 deletion Nhl.Api.Common/Http/NhlSuggestionApiHttpClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
public class NhlSuggestionApiHttpClient : NhlApiHttpClient
{
private static readonly object _lock = new object();
private static HttpClient _httpClient;

Check warning on line 11 in Nhl.Api.Common/Http/NhlSuggestionApiHttpClient.cs

View workflow job for this annotation

GitHub Actions / build

Non-nullable field '_httpClient' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the field as nullable.

/// <summary>
/// The NHL HTTP API endpoint for the NHL suggestion API
Expand All @@ -18,7 +18,7 @@
/// <summary>
/// The dedicated NHL HTTP Client for the NHL suggestion API
/// </summary>
public NhlSuggestionApiHttpClient() : base(clientApiUri: ClientApiUrl, clientVersion: "v1", timeoutInSeconds: 30)
public NhlSuggestionApiHttpClient() : base(clientApiUri: ClientApiUrl, clientVersion: "v1", timeoutInSeconds: 60)
{
}

Expand Down
9 changes: 7 additions & 2 deletions Nhl.Api.Domain/Models/Game/GameCenterPlayByPlay.cs
Original file line number Diff line number Diff line change
Expand Up @@ -294,8 +294,7 @@ public class GameCenterPlay
/// The period number for the NHL game center play by play <br/>
/// Example: 1
/// </summary>
[JsonProperty("period")]
public int Period { get; set; }
public int Period => this.PeriodDescriptor?.Number ?? 0;

/// <summary>
/// The period descriptor for the NHL game center play by play <br/>
Expand Down Expand Up @@ -364,6 +363,12 @@ public class GameCenterPlay
/// </summary>
[JsonProperty("details")]
public GameCenterDetails Details { get; set; }

/// <summary>
/// The estimated time of the play for the NHL game center play by play, these are not exact times and are close approximations of each game event <br/>
/// Example: 2024-01-13T20:12:23Z
/// </summary>
public DateTimeOffset? EstimatedDateTimeOfPlay { get; set; }
}

/// <summary>
Expand Down
31 changes: 30 additions & 1 deletion Nhl.Api.Domain/Models/Season/SeasonYears.cs
Original file line number Diff line number Diff line change
@@ -1,11 +1,40 @@


using System.Collections.Generic;

namespace Nhl.Api.Models.Season;

/// <summary>
/// All of the NHL season years since the inception of the NHL league
/// </summary>
public class SeasonYear
{
/// <summary>
/// A collection of all the NHL seasons
/// </summary>
public static readonly HashSet<string> AllSeasons =
[
"19171918", "19181919", "19191920", "19201921", "19211922", "19221923",
"19231924", "19241925", "19251926", "19261927", "19271928", "19281929",
"19291930", "19301931", "19311932", "19321933", "19331934", "19341935",
"19351936", "19361937", "19371938", "19381939", "19391940", "19401941",
"19411942", "19421943", "19431944", "19441945", "19451946", "19461947",
"19471948", "19481949", "19491950", "19501951", "19511952", "19521953",
"19531954", "19541955", "19551956", "19561957", "19571958", "19581959",
"19591960", "19601961", "19611962", "19621963", "19631964", "19641965",
"19651966", "19661967", "19671968", "19681969", "19691970", "19701971",
"19711972", "19721973", "19731974", "19741975", "19751976", "19761977",
"19771978", "19781979", "19791980", "19801981", "19811982", "19821983",
"19831984", "19841985", "19851986", "19861987", "19871988", "19881989",
"19891990", "19901991", "19911992", "19921993", "19931994", "19941995",
"19951996", "19961997", "19971998", "19981999", "19992000", "20002001",
"20012002", "20022003", "20032004", "20052006", "20062007", "20072008",
"20082009", "20092010", "20102011", "20112012", "20122013", "20132014",
"20142015", "20152016", "20162017", "20172018", "20182019", "20192020",
"20202021", "20212022", "20222023", "20232024", "20242025", "20252026",
"20262027", "20272028", "20282029", "20292030", "20302031", "20312032",
"20322033", "20332034", "20342035"
];

/// <summary>
/// The NHL season 1917-1918
/// </summary>
Expand Down
178 changes: 176 additions & 2 deletions Nhl.Api.Domain/Services/NhlGameService.cs
Original file line number Diff line number Diff line change
@@ -1,16 +1,190 @@
namespace Nhl.Api.Services;
using System.Collections.Generic;
using System.Globalization;
using System.Text.RegularExpressions;
using System;
using System.Threading.Tasks;
using Nhl.Api.Common.Http;
using Nhl.Api.Models.Game;
using System.Linq;
using Nhl.Api.Models.Schedule;

namespace Nhl.Api.Services;

/// <summary>
/// The NHL game service, enabling data enrichment and functionality to the Nhl.Api
/// </summary>
public interface INhlGameService
{

/// <summary>
/// A method to add the time of play for each play in the game center play by play
/// </summary>
/// <param name="gameCenterPlayByPlay"> A game center play by play object</param>
/// <returns>All of the game center play by play information with the time of play for each play</returns>
Task<GameCenterPlayByPlay> AddEstimatedDateTimeOfPlayForEachPlay(GameCenterPlayByPlay gameCenterPlayByPlay);
}

/// <summary>
/// The NHL game service, enabling data enrichment and functionality to the Nhl.Api
/// </summary>
public class NhlGameService : INhlGameService
{
private readonly NhlScoresHtmlReportsApiHttpClient _nhlScoresHtmlReportsApiHttpClient = new();

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Inject the HTTP client via dependency injection.

The _nhlScoresHtmlReportsApiHttpClient is instantiated directly within the NhlGameService class. This tight coupling makes unit testing more difficult and violates the Dependency Inversion Principle.

Consider injecting the NhlScoresHtmlReportsApiHttpClient through the constructor or using an interface to allow for better testability and adherence to SOLID principles.

/// <summary>
/// A method to add the estimated time of play for each play in the game center play by play
/// </summary>
/// <param name="gameCenterPlayByPlay"> A game center play by play object</param>
/// <returns>All of the game center play by play information with the time of play for each play</returns>
public async Task<GameCenterPlayByPlay> AddEstimatedDateTimeOfPlayForEachPlay(GameCenterPlayByPlay gameCenterPlayByPlay)
{
var gameId = gameCenterPlayByPlay.Id.ToString(CultureInfo.InvariantCulture)[4..];
var scoreReport = await this._nhlScoresHtmlReportsApiHttpClient.GetStringAsync($"/{gameCenterPlayByPlay.Season}/PL{gameId}.HTM", default);

var timePeriodMatches = Regex.Matches(scoreReport, @"(?<=<td class="" \+ bborder"">)Period(.*?)(?=</td>)", RegexOptions.Compiled | RegexOptions.CultureInvariant, TimeSpan.FromSeconds(5)).ToList();

if (string.IsNullOrWhiteSpace(scoreReport) || timePeriodMatches.Count == 0)
{
try
{
// Parse game start time and adjust for venue UTC offset
var gameStart = DateTimeOffset.Parse($"{gameCenterPlayByPlay.StartTimeUTC}", CultureInfo.InvariantCulture);
var timeSpanOffset = TimeSpan.Parse(gameCenterPlayByPlay.VenueUTCOffset, CultureInfo.InvariantCulture);

gameStart = gameStart.AddHours(timeSpanOffset.TotalHours); // Adjust with venue UTC offset

// Add average delay in start time (assuming game starts on average 8.5 minutes after scheduled time)
gameStart = gameStart.AddMinutes(8.5);

// Constants based on estimated NHL standards
var averagePeriodDuration = TimeSpan.FromMinutes(42.5); // Average period duration including stoppages
var intermissionDuration = TimeSpan.FromMinutes(20); // Intermission duration
var overtimeIntermissionDuration = TimeSpan.FromMinutes(2.5); // Short break before OT
var maxRegularPeriods = 3; // Number of regular periods

foreach (var play in gameCenterPlayByPlay.Plays)
{
var period = play.Period;
var timeElapsed = TimeSpan.Zero;
var timeInPeriod = TimeSpan.ParseExact(play.TimeInPeriod, @"mm\:ss", CultureInfo.InvariantCulture);

if (period <= maxRegularPeriods)
{
// Regular periods
var completedPeriods = period - 1;

// Total time from completed periods and intermissions
timeElapsed += completedPeriods * (averagePeriodDuration + intermissionDuration);

// Time elapsed within the current period
timeElapsed += timeInPeriod;
}
else
{
// Overtime periods
// Time from regular periods and intermissions
timeElapsed += maxRegularPeriods * (averagePeriodDuration + intermissionDuration);

// Add short break before overtime
timeElapsed += overtimeIntermissionDuration;

// Time within overtime period
timeElapsed += timeInPeriod;
}

// Calculate the estimated DateTime of the play
var estimatedDateTime = gameStart.Add(timeElapsed);

// Assign estimated date time to play
play.EstimatedDateTimeOfPlay = estimatedDateTime;
}
}
catch
{
}
}
else
{
try
{
var startTimeOfEachPeriod = new Dictionary<int, List<DateTime>>
{
{ 1, [] },
{ 2, [] },
{ 3, [] },
{ 4, [] },
{ 5, [] },
};

for (var i = 0; i < timePeriodMatches.Count; i++)
{
var match = timePeriodMatches[i].Value;
var value = Regex.Match(match, @"([0-9]{1,2}:[0-9]{2})", RegexOptions.Compiled | RegexOptions.IgnoreCase, TimeSpan.FromSeconds(30)).Groups[0].Value;
var time = TimeOnly.Parse($"{value} PM", CultureInfo.InvariantCulture);
var dateTime = DateTime.Parse($"{gameCenterPlayByPlay.GameDate} {time} {gameCenterPlayByPlay.EasternUTCOffset}", CultureInfo.InvariantCulture);

if (i <= 1)
{
startTimeOfEachPeriod[1].Add(dateTime);
}
else if (i is >= 2 and <= 3)
{
startTimeOfEachPeriod[2].Add(dateTime);
}
else if (i is >= 4 and <= 5)
{
startTimeOfEachPeriod[3].Add(dateTime);
}
else if (i is >= 6 and <= 7)
{
startTimeOfEachPeriod[4].Add(dateTime);
}
else if (i <= 9)
{
startTimeOfEachPeriod[5].Add(dateTime);
}
}

var playsByPeriod = gameCenterPlayByPlay.Plays.GroupBy(x => x.Period).ToDictionary(x => x.Key, x => x.ToList());

foreach (var playsForPeriod in playsByPeriod)
{
var startTime = startTimeOfEachPeriod[playsForPeriod.Key].FirstOrDefault();
var endTime = startTimeOfEachPeriod[playsForPeriod.Key].LastOrDefault();
if (startTime == default || endTime == default)
{
continue;
}

var distanceBetweenPlays = endTime - startTime;
var timeBetweenPlays = distanceBetweenPlays / playsForPeriod.Value.Count;

var multiplier = this.CalculateMultiplier(startTime, endTime, playsForPeriod.Value.Count);
for (var i = 0; i < playsForPeriod.Value.Count; i++)
{
var play = playsForPeriod.Value[i];
var timeElapsed = TimeOnly.Parse($"00:{playsForPeriod.Value[i].TimeInPeriod}", CultureInfo.InvariantCulture).ToTimeSpan();
play.EstimatedDateTimeOfPlay = startTime.Add(timeElapsed + (timeBetweenPlays * (i * multiplier)));
}
}
}
catch
{
}
}

return gameCenterPlayByPlay;
}

/// <summary>
/// A linear
/// </summary>
private double CalculateMultiplier(DateTime startTime, DateTime endTime, int events)
{
// Calculate duration in minutes
var duration = (endTime - startTime).TotalMinutes;

var multiplier = ((41 * duration) + (2 * events) - 381) / 3000;

return multiplier;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve documentation and extract magic numbers in CalculateMultiplier.

The method's documentation is incomplete, and the formula uses magic numbers without explanation.

 /// <summary>
-/// A linear 
+/// Calculates a multiplier for time estimation based on period duration and number of events.
+/// The multiplier is used to adjust the distribution of plays within a period.
 /// </summary>
+/// <param name="startTime">The start time of the period</param>
+/// <param name="endTime">The end time of the period</param>
+/// <param name="events">The number of events in the period</param>
+/// <returns>A multiplier value for time distribution calculations</returns>
 private double CalculateMultiplier(DateTime startTime, DateTime endTime, int events)
 {
+    // Constants for the multiplier formula
+    private const double DURATION_COEFFICIENT = 41.0;
+    private const double EVENTS_COEFFICIENT = 2.0;
+    private const double CONSTANT_TERM = 381.0;
+    private const double NORMALIZER = 3000.0;
+
     // Calculate duration in minutes
     var duration = (endTime - startTime).TotalMinutes;
 
-    var multiplier = ((41 * duration) + (2 * events) - 381) / 3000;
+    var multiplier = ((DURATION_COEFFICIENT * duration) + 
+                     (EVENTS_COEFFICIENT * events) - 
+                     CONSTANT_TERM) / NORMALIZER;
 
     return multiplier;
 }

Committable suggestion skipped: line range outside the PR's diff.

28 changes: 15 additions & 13 deletions Nhl.Api.Tests/GameTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -210,43 +210,45 @@ public async Task GetGameMetadataByGameIdAsync_Return_Valid_Information(int game
[TestMethodWithRetry(RetryCount = 5)]
public async Task NhlScoresHtmlReportsApiHttpClient_Can_Parse_Html_Report_For_Start_End_Period_Times()
{
var dictionary = new Dictionary<string, List<string>>
var dictionary = new Dictionary<string, List<TimeOnly>>
{
{ "P1", new List<string>() },
{ "P2", new List<string>() },
{ "P3", new List<string>() },
{ "OT", new List<string>() },
{ "SH", new List<string>() },
{ "P1", new List<TimeOnly>() },
{ "P2", new List<TimeOnly>() },
{ "P3", new List<TimeOnly>() },
{ "OT", new List<TimeOnly>() },
{ "SH", new List<TimeOnly>() },
};

var httpClient = new NhlScoresHtmlReportsApiHttpClient();
var gameReport = await httpClient.GetStringAsync("/20232024/PL020206.HTM");

var regex = Regex.Matches(gameReport, @"(?<=<td class="" \+ bborder"">)Period(.*?)(?=</td>)", RegexOptions.Compiled, TimeSpan.FromSeconds(30)).ToList();

for (int i = 0; i < regex.Count; i++)
for (var i = 0; i < regex.Count; i++)
{
var match = regex[i].Value;
var value = Regex.Match(match, @"([0-9]{1,2}:[0-9]{2}\s[A-Z]{3})", RegexOptions.Compiled | RegexOptions.IgnoreCase, TimeSpan.FromSeconds(30)).Groups[0].Value;
var value = Regex.Match(match, @"([0-9]{1,2}:[0-9]{2})", RegexOptions.Compiled | RegexOptions.IgnoreCase, TimeSpan.FromSeconds(30)).Groups[0].Value;
var time = TimeOnly.Parse($"{value} PM");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve time parsing robustness

The current implementation assumes all times are PM, which may not always be correct. To improve robustness, consider the following approaches:

  1. If possible, extract the AM/PM information from the HTML report.
  2. Use game start time to determine the correct AM/PM for each period.
  3. Implement logic to handle time rollovers (e.g., from PM to AM for late-night games).

Example implementation:

var gameStartTime = ExtractGameStartTime(gameReport); // Implement this method
var time = ParseTimeWithContext(value, gameStartTime);

// ...

private TimeOnly ParseTimeWithContext(string timeString, TimeOnly gameStartTime)
{
    var parsedTime = TimeOnly.Parse(timeString);
    if (parsedTime < gameStartTime && gameStartTime.Hour >= 12)
    {
        // If parsed time is earlier than game start and game started in PM, assume next day
        parsedTime = parsedTime.AddHours(12);
    }
    return parsedTime;
}

This approach would provide more accurate time representations for games spanning different times of day.


if (i <= 1)
{
dictionary["P1"].Add(value);
dictionary["P1"].Add(time);
}
else if (i >= 2 && i <= 3)
{
dictionary["P2"].Add(value);
dictionary["P2"].Add(time);
}
else if (i >= 4 && i <= 5)
{
dictionary["P3"].Add(value);
dictionary["P3"].Add(time);
}
else if (i >= 6 && i <= 7)
{
dictionary["OT"].Add(value);
dictionary["OT"].Add(time);
}
else if (i <= 9)
{
dictionary["SH"].Add(value);
dictionary["SH"].Add(time);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider adding error handling and validation

While the overall structure of the method is sound and the changes improve type safety, consider adding the following enhancements:

  1. Error handling for regex matches and time parsing.
  2. Validation of the extracted times (e.g., ensure they are in a logical sequence).
  3. Logging of any parsing issues or unexpected data.

Example:

try
{
    var time = TimeOnly.Parse($"{value} PM");
    if (time < previousTime)
    {
        // Log warning about unexpected time sequence
    }
    // ... existing code ...
}
catch (FormatException ex)
{
    // Log error about invalid time format
    // Consider how to handle this case (skip, use default value, etc.)
}

These additions would make the method more robust and easier to debug in case of unexpected data or parsing issues.

}
}
}
Expand Down
Loading
Loading