v3.5.0

[Afischbacher]

No description provided.

[coderabbitai]

📝 Walkthrough

📝 Walkthrough

Walkthrough

This pull request introduces two new asynchronous methods to the NHL API, enhancing the functionality for retrieving player statistics. The methods, GetRealtimePlayerStatisticsBySeasonAndFilterExpressionAsync and GetTimeOnIcePlayerStatisticsBySeasonAndFilterExpressionAsync, allow developers to access real-time and time-on-ice statistics for players during specific seasons, with filtering and sorting options. Additionally, nullable return types have been implemented in the NhlTeamService interface, and the retry mechanism in the testing attributes has been improved. The README has been updated to reflect these changes.

Changes

Files	Change Summary
Nhl.Api/README.md	Added documentation for new asynchronous methods related to player statistics.
Nhl.Api.Domain/Services/NhlTeamService.cs	Updated method signatures to nullable types and added handling for TeamEnum.UtahHockeyClub.
Nhl.Api/Src/GameApi/NhlGameApi.cs	Modified GetGameCenterBoxScoreByGameIdAsync to perform concurrent data retrieval using asynchronous HTTP requests.
Nhl.Api.Tests/Helpers/Attributes/TestMethodWithRetryAttribute.cs	Enhanced retry functionality with a new backoff strategy and updated default retry delay.
Nhl.Api.Tests/TeamTests.cs	Reordered using directives for clarity.
Nhl.Api.Tests/StatisticsTests.cs	Added multiple new test methods to validate player and goalie statistics across various seasons, including edge cases.

Possibly related PRs

• v3.4.2 #56: The changes in this PR involve updates to player-related functionalities, including modifications to player data structures, which may relate to the new asynchronous methods for retrieving player statistics in the main PR.

Poem

🐇 In the realm of stats so bright,
New methods shine, a wondrous sight!
Real-time numbers, ice-time too,
For players' feats, we now have a view.
Hooray for hockey, let’s cheer and play,
With stats at our paws, hip-hip-hooray! 🏒

---

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[coderabbitai]

Actionable comments posted: 17

Outside diff range and nitpick comments (18)

Nhl.Api.Common/Nhl.Api.Common.csproj (1)

Line range hint 12-12: Consider updating Newtonsoft.Json package

While not directly related to the version change, I noticed that the Newtonsoft.Json package is not at the latest version. Consider updating it to the latest stable version (13.0.3 as of September 2024) to ensure you have the latest bug fixes and performance improvements.

You can update the package by modifying the following line:

-		<PackageReference Include="Newtonsoft.Json" Version="13.0.1" />
+		<PackageReference Include="Newtonsoft.Json" Version="13.0.3" />

Nhl.Api.Domain/Models/Player/StatisticsReportType.cs (1)

6-9: Fix typo in documentation and LGTM for enum declaration.

The enum declaration and its documentation look good. However, there's a small typo in the documentation comment:

- /// A statistics filter for the retreival of player and goalie statistics in the NHL using Cayenne expressions
+ /// A statistics filter for the retrieval of player and goalie statistics in the NHL using Cayenne expressions

Please correct this typo to maintain the quality of the documentation.

Nhl.Api.Domain/Models/Statistics/PlayerTimeOnIceStatisticsFilterResult.cs (1)

1-3: Consider adding a license header to the file.

It's a good practice to include a license header at the top of each source file in open-source projects. This helps to clearly communicate the terms under which the code is distributed. Consider adding a license header similar to:

// Copyright (c) [year] [copyright holders]
// SPDX-License-Identifier: [license identifier]
// See the LICENSE file in the project root for full license information.

Replace [year], [copyright holders], and [license identifier] with the appropriate information for your project.

Nhl.Api.Domain/Models/Statistics/GoalieStatisticsFilterResult.cs (1)

Line range hint 94-153: Summary of changes and suggestion for documentation

The changes in this PR (v3.5.0) consistently update four properties in the GoalieStatisticsResult class to allow for nullable values:

1. OvertimeLosses (int? instead of int)
2. SavePercentage (decimal? instead of double)
3. Saves (int? instead of int)
4. ShotsAgainst (int? instead of int)

These changes improve the flexibility of the data model by allowing for scenarios where certain statistics might not be available. However, this could potentially impact how consumers of this API handle these properties, as they now need to account for null values.

Consider updating the API documentation or release notes to highlight these changes. This will help API consumers understand the new nullable nature of these properties and update their code accordingly to handle potential null values.

Nhl.Api.Domain/Models/Statistics/PlayerStatisticsFilterResult.cs (1)

210-211: Remove unnecessary empty lines

There are two empty lines at the end of the file. It's generally a good practice to end a file with a single newline character. Consider removing one of these empty lines to adhere to common coding standards.

 }
-

Nhl.Api.Domain/Models/Statistics/PlayerRealtimeStatisticsFilterResult.cs (1)

24-203: LGTM: Comprehensive player statistics class with proper JSON serialization.

The PlayerRealtimeStatisticsResult class is well-implemented, covering a wide range of player statistics with appropriate use of nullable types and consistent JSON serialization attributes. The property names and documentation are clear and follow C# conventions.

For consistency, consider making the GiveawaysPer60, HitsPer60, and TakeawaysPer60 properties nullable (double?) like other similar properties. This would maintain consistency with other statistical properties and allow for the possibility of missing data.

Example:

[JsonProperty("giveawaysPer60")]
public double? GiveawaysPer60 { get; set; }

Nhl.Api.Domain/Models/Team/TeamCodes.cs (1)

241-244: LGTM! Consider adding more context about the team's status.

The addition of the UtahHockeyClub constant is correct and consistent with the existing code style. The three-letter code "UTA" follows the established pattern for team identifiers.

Consider adding a brief note about the team's current status or expected start date in the summary comment. For example:

/// <summary>
/// Utah Hockey Club (Expected to join in 20XX or: Placeholder for future expansion)
/// </summary>
public const string UtahHockeyClub = "UTA";

This additional context could be helpful for developers using this constant in the future.

Nhl.Api/Src/StatisticsApi/INhlStatisticsApi.cs (1)

141-151: LGTM with a minor suggestion.

The new method GetRealtimePlayerStatisticsBySeasonAndFilterExpressionAsync is well-documented and consistent with the interface's style. However, consider changing the default value of playerRealtimeStatisticsFilterToSortBy to a more commonly used statistic, such as Goals or Points, instead of OvertimeGoals.

-public Task<PlayerRealtimeStatisticsFilterResult> GetRealtimePlayerStatisticsBySeasonAndFilterExpressionAsync(string seasonYear, ExpressionPlayerFilter expressionPlayerFilter, PlayerRealtimeStatisticsFilter playerRealtimeStatisticsFilterToSortBy = PlayerRealtimeStatisticsFilter.OvertimeGoals, int limit = -1, int offsetStart = 0, CancellationToken cancellationToken = default);
+public Task<PlayerRealtimeStatisticsFilterResult> GetRealtimePlayerStatisticsBySeasonAndFilterExpressionAsync(string seasonYear, ExpressionPlayerFilter expressionPlayerFilter, PlayerRealtimeStatisticsFilter playerRealtimeStatisticsFilterToSortBy = PlayerRealtimeStatisticsFilter.Goals, int limit = -1, int offsetStart = 0, CancellationToken cancellationToken = default);

Nhl.Api.Tests/LeagueTests.cs (2)

382-382: Document the reason for changing from TeamEnum to teamId.

The method signature has been updated to use int teamId instead of TeamEnum team, and the method name now includes "With_TeamId". While this change might be part of a larger refactoring effort, it's not immediately clear why this change was made.

Consider adding a comment explaining the rationale behind this change. This will help future maintainers understand the decision-making process and the potential implications of this change.

Example comment:

// Changed from TeamEnum to int teamId to align with the new team identification strategy across the API.
// This allows for more flexibility in team representation and better integration with external systems.
public async Task GetTeamStatisticsBySeasonAndGameTypeAsync_Return_Valid_Information_With_TeamId(int teamId, string seasonYear, GameType gameType)

---

344-382: Consider documenting the overall refactoring strategy.

The changes in this file appear to be part of a larger refactoring effort, particularly in how teams are identified and how tests are structured. While these changes may be necessary, there are a few concerns:

1. The significant reduction in test coverage by commenting out multiple test cases.
2. The shift from using TeamEnum to int teamId without clear documentation.
3. Lack of overall context for these changes.

To improve the maintainability and understandability of the code:

1. Document the overall refactoring strategy, perhaps in a comment at the top of the file or in a separate document referenced in the code.
2. Reconsider the approach to test case reduction. If possible, update and re-enable the commented-out tests to maintain good coverage.
3. Add comments explaining significant changes, such as the switch from TeamEnum to int teamId.

Consider creating a separate document or wiki page that outlines the refactoring strategy, including the rationale for changes, the expected benefits, and any known trade-offs. This will provide valuable context for current and future developers working on this codebase.

Nhl.Api/Src/LeagueApi/NhlLeagueApi.cs (1)

413-418: LGTM! Consider adding a comment about the team's status.

The addition of the Utah Hockey Club team colors is implemented correctly and follows the existing pattern. The color scheme (blue, black, white) seems appropriate for a hockey team.

Consider adding a comment above this case to indicate the status of the Utah Hockey Club (e.g., if it's a new expansion team or a rebranded existing team). This would provide helpful context for future maintainers.

Nhl.Api/README.md (2)

1009-1030: Minor inconsistency in parameter description

The documentation for this new method looks good and is consistent with the existing style. However, there's a small inconsistency in the parameter description for playerRealtimeStatisticsFilterToSortBy. The description references PlayerStatisticsFilter instead of PlayerRealtimeStatisticsFilter.

Consider updating the parameter description as follows:

-| playerRealtimeStatisticsFilterToSortBy | [Nhl.Api.Models.Player.PlayerRealtimeStatisticsFilter](#T-Nhl-Api-Models-Player-PlayerRealtimeStatisticsFilter 'Nhl.Api.Models.Player.PlayerRealtimeStatisticsFilter') | The player statistics filter to sort the player statistics by, see [PlayerStatisticsFilter](#T-Nhl-Api-Models-Player-PlayerStatisticsFilter 'Nhl.Api.Models.Player.PlayerStatisticsFilter') for more information on valid player statistics filters |
+| playerRealtimeStatisticsFilterToSortBy | [Nhl.Api.Models.Player.PlayerRealtimeStatisticsFilter](#T-Nhl-Api-Models-Player-PlayerRealtimeStatisticsFilter 'Nhl.Api.Models.Player.PlayerRealtimeStatisticsFilter') | The player real-time statistics filter to sort the player statistics by, see [PlayerRealtimeStatisticsFilter](#T-Nhl-Api-Models-Player-PlayerRealtimeStatisticsFilter 'Nhl.Api.Models.Player.PlayerRealtimeStatisticsFilter') for more information on valid player real-time statistics filters |

This change will make the documentation more accurate and consistent.

Overall, the addition of this method is well-documented and follows the existing style.

Tools

Markdownlint

1012-1012: Expected: h4; Actual: h5
Heading levels should only increment by one level at a time

(MD001, heading-increment)

---

1024-1024: null
Link fragments should be valid

(MD051, link-fragments)

---

1025-1025: null
Link fragments should be valid

(MD051, link-fragments)

---

1025-1025: null
Link fragments should be valid

(MD051, link-fragments)

---

1026-1026: null
Link fragments should be valid

(MD051, link-fragments)

---

1026-1026: null
Link fragments should be valid

(MD051, link-fragments)

---

2989-3010: Suggestion for improved method description

The documentation for this new method is well-structured and consistent with the existing style. However, the method description could be slightly improved for clarity.

Consider updating the method description as follows:

-Returns all the NHL time on ice player game center statistics for a specific player for a specific season including even time on ice, overtime time on ice, over time on ice per over time game time on ice per game and more
+Returns all the NHL time on ice player game center statistics for a specific season, including even strength time on ice, overtime time on ice, time on ice per game, and more

This change makes the description more concise and easier to read while still conveying the key information about the method's functionality.

Overall, the addition of this method is well-documented and follows the existing style.

Tools

Markdownlint

2992-2992: Expected: h4; Actual: h5
Heading levels should only increment by one level at a time

(MD001, heading-increment)

---

3004-3004: null
Link fragments should be valid

(MD051, link-fragments)

---

3005-3005: null
Link fragments should be valid

(MD051, link-fragments)

---

3005-3005: null
Link fragments should be valid

(MD051, link-fragments)

---

3006-3006: null
Link fragments should be valid

(MD051, link-fragments)

---

3006-3006: null
Link fragments should be valid

(MD051, link-fragments)

Nhl.Api.Domain/Models/Player/ExpressionPlayerFilterBuilder.cs (2)

Line range hint 129-236: Ensure proper handling of user-provided input to prevent injection vulnerabilities

In the comparison methods (EqualTo, NotEqualTo, GreaterThan, GreaterThanOrEqualTo, LessThan, LessThanOrEqualTo), user-provided value is appended directly to the _filterExpression without sanitization. This could lead to injection vulnerabilities if value contains malicious input.

Consider sanitizing or validating value before appending it to the expression. Alternatively, implement parameterization to securely handle user-provided values.

---

483-845: Consider consolidating common statistics into a base enum to reduce duplication

Several statistics, such as GamesPlayed, LastName, PlayerId, SeasonId, ShootsCatches, SkaterFullName, and TeamAbbreviation, appear in multiple enums (PlayerStatisticsFilter, PlayerRealtimeStatisticsFilter, PlayerTimeOnIceStatisticsFilter).

Consider consolidating these common statistics into a shared base enum or interface to reduce code duplication and improve maintainability.

Nhl.Api/Src/StatisticsApi/NhlStatisticsApi.cs (3)

Line range hint 880-895: Bug in 'takeaway' case: Incorrect statistic increment

In the GetPlayerStatisticTotal method, within the takeaway case, the code incorrectly increments the Giveaway statistic instead of the Takeaway statistic. This leads to inaccurate statistics being recorded.

Apply the following diff to fix the issue:

                 case "takeaway":
                     if (play.Details.PlayerId.HasValue)
                     {
-                        gameStatisticTotals[PlayerGameCenterStatistic.Takeaway] = play.Details.PlayerId == playerId
-                            ? gameStatisticTotals[PlayerGameCenterStatistic.Giveaway] += 1
-                            : gameStatisticTotals[PlayerGameCenterStatistic.Giveaway];
+                        gameStatisticTotals[PlayerGameCenterStatistic.Takeaway] = play.Details.PlayerId == playerId
+                            ? gameStatisticTotals[PlayerGameCenterStatistic.Takeaway] += 1
+                            : gameStatisticTotals[PlayerGameCenterStatistic.Takeaway];
                     }
                     break;

---

Line range hint 733-736: Typo in variable name 'teamAbbreviaton'

The variable teamAbbreviaton seems to have a typographical error. It should be teamAbbreviation.

Apply the following diff to correct the typo:

-    var teamAbbreviaton = _nhlTeamService.GetTeamCodeIdentifierByTeamId(teamId);
+    var teamAbbreviation = _nhlTeamService.GetTeamCodeIdentifierByTeamId(teamId);

Ensure to update all occurrences of this variable name throughout the code.

---

Line range hint 734-737: Possible NullReferenceException risk

In the method GetTeamStatisticsBySeasonAndGameTypeAsync, after retrieving teamAbbreviation, the code directly uses it without checking for null or empty values. If _nhlTeamService.GetTeamCodeIdentifierByTeamId(teamId) returns null, this could lead to a NullReferenceException.

Consider adding a check to ensure teamAbbreviation is not null or empty before using it:

+    if (string.IsNullOrWhiteSpace(teamAbbreviation))
+    {
+        throw new ArgumentException("Invalid team ID provided.");
+    }

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 4366edf and bcc9d49.

Files selected for processing (24)

• Nhl.Api.Common/Helpers/TeamNames.cs (2 hunks)
• Nhl.Api.Common/Nhl.Api.Common.csproj (1 hunks)
• Nhl.Api.Domain/Enumerations/Franchise/FranchiseEnum.cs (2 hunks)
• Nhl.Api.Domain/Enumerations/Team/TeamEnum.cs (2 hunks)
• Nhl.Api.Domain/Models/Player/ExpressionPlayerFilterBuilder.cs (13 hunks)
• Nhl.Api.Domain/Models/Player/StatisticsReportType.cs (1 hunks)
• Nhl.Api.Domain/Models/Statistics/GoalieStatisticsFilterResult.cs (3 hunks)
• Nhl.Api.Domain/Models/Statistics/PlayerRealtimeStatisticsFilterResult.cs (1 hunks)
• Nhl.Api.Domain/Models/Statistics/PlayerStatisticsFilterResult.cs (6 hunks)
• Nhl.Api.Domain/Models/Statistics/PlayerTimeOnIceStatisticsFilterResult.cs (1 hunks)
• Nhl.Api.Domain/Models/Team/TeamCodes.cs (2 hunks)
• Nhl.Api.Domain/Nhl.Api.Domain.csproj (1 hunks)
• Nhl.Api.Domain/Services/NhlTeamService.cs (5 hunks)
• Nhl.Api.Tests/LeagueTests.cs (1 hunks)
• Nhl.Api.Tests/Nhl.Api.Tests.csproj (1 hunks)
• Nhl.Api.Tests/ProjectStructureTests.cs (2 hunks)
• Nhl.Api.Tests/StatisticsTests.cs (7 hunks)
• Nhl.Api.Tests/TeamTests.cs (1 hunks)
• Nhl.Api/Nhl.Api.csproj (2 hunks)
• Nhl.Api/README.md (7 hunks)
• Nhl.Api/Src/Api/NhlApi.cs (2 hunks)
• Nhl.Api/Src/LeagueApi/NhlLeagueApi.cs (1 hunks)
• Nhl.Api/Src/StatisticsApi/INhlStatisticsApi.cs (2 hunks)
• Nhl.Api/Src/StatisticsApi/NhlStatisticsApi.cs (2 hunks)

Files skipped from review due to trivial changes (1)

• Nhl.Api.Tests/TeamTests.cs

Additional context used

Markdownlint

Nhl.Api/README.md

1012-1012: Expected: h4; Actual: h5
Heading levels should only increment by one level at a time

(MD001, heading-increment)

---

1024-1024: null
Link fragments should be valid

(MD051, link-fragments)

---

1025-1025: null
Link fragments should be valid

(MD051, link-fragments)

---

1025-1025: null
Link fragments should be valid

(MD051, link-fragments)

---

1026-1026: null
Link fragments should be valid

(MD051, link-fragments)

---

1026-1026: null
Link fragments should be valid

(MD051, link-fragments)

---

1450-1450: Expected: h4; Actual: h5
Heading levels should only increment by one level at a time

(MD001, heading-increment)

---

1462-1462: null
Link fragments should be valid

(MD051, link-fragments)

---

1463-1463: null
Link fragments should be valid

(MD051, link-fragments)

---

1463-1463: null
Link fragments should be valid

(MD051, link-fragments)

---

1464-1464: null
Link fragments should be valid

(MD051, link-fragments)

---

1464-1464: null
Link fragments should be valid

(MD051, link-fragments)

---

2873-2873: Expected: h4; Actual: h5
Heading levels should only increment by one level at a time

(MD001, heading-increment)

---

2885-2885: null
Link fragments should be valid

(MD051, link-fragments)

---

2886-2886: null
Link fragments should be valid

(MD051, link-fragments)

---

2886-2886: null
Link fragments should be valid

(MD051, link-fragments)

---

2887-2887: null
Link fragments should be valid

(MD051, link-fragments)

---

2887-2887: null
Link fragments should be valid

(MD051, link-fragments)

---

2992-2992: Expected: h4; Actual: h5
Heading levels should only increment by one level at a time

(MD001, heading-increment)

---

3004-3004: null
Link fragments should be valid

(MD051, link-fragments)

---

3005-3005: null
Link fragments should be valid

(MD051, link-fragments)

---

3005-3005: null
Link fragments should be valid

(MD051, link-fragments)

---

3006-3006: null
Link fragments should be valid

(MD051, link-fragments)

---

3006-3006: null
Link fragments should be valid

(MD051, link-fragments)

Additional comments not posted (45)

Nhl.Api.Common/Nhl.Api.Common.csproj (1)

4-4: Version update looks good!

The version has been correctly updated from 3.4.2 to 3.5.0, which aligns with the PR title "v3.5.0". This minor version increment suggests new features or improvements while maintaining backward compatibility.

Nhl.Api.Domain/Models/Player/StatisticsReportType.cs (2)

1-4: LGTM: Appropriate using statements and namespace.

The using statements are correctly included for the attributes used in the enum, and the namespace follows the expected structure based on the file path.

---

1-30: Overall assessment: Well-implemented enum for NHL statistics filtering.

This new StatisticsReportType enum is a valuable addition to the Nhl.Api.Models.Player namespace. It provides a type-safe way to specify different types of statistics reports for NHL players and goalies, which aligns well with the objectives of the v3.5.0 release.

The implementation is generally well-structured, following C# conventions and making good use of attributes for serialization and descriptions. The minor suggestions provided (fixing a typo and considering the use of constants for string values) would further enhance the code quality and maintainability.

Great job on this addition to the API!

Nhl.Api.Domain/Nhl.Api.Domain.csproj (1)

4-4: Version update looks good.

The version has been correctly updated from 3.4.2 to 3.5.0, which aligns with the PR title. This minor version increment suggests new features or improvements that are backwards compatible.

To ensure consistency across the project, please run the following script:

This script will help verify:

1. Any other occurrences of the old or new version number in C# files.
2. If CHANGELOG.md has been updated with the new version.
3. If AssemblyInfo.cs (if it exists) has been updated with the new version.

Please review the results and ensure all necessary updates have been made.

Nhl.Api.Tests/Nhl.Api.Tests.csproj (2)

4-4: Verify justification for minor version bump.

While the version update itself looks good, it's important to ensure that this minor version increase (3.4.x to 3.5.0) is justified by new features or significant enhancements in the codebase.

Could you please provide a brief summary of the new features or significant changes that warrant this minor version bump? This information will be valuable for maintaining a clear changelog and helping users understand what's new in this version.

---

4-4: Version update looks good.

The version has been correctly updated from 3.4.2 to 3.5.0, which aligns with the PR title. This change follows semantic versioning practices, where a minor version increase typically indicates new features added in a backwards-compatible manner.

Let's verify the version consistency across the project and check for corresponding changes:

This script will help us ensure that the version update is consistent across the project and that there are changes justifying the minor version bump.

Verification successful

This script will help us get a better understanding of the codebase structure, recent modifications, and public API surface, which can indicate potential new features or changes justifying the minor version bump.

---

Version update to 3.5.0 is justified and consistent.

The minor version bump from 3.4.x to 3.5.0 appears to be warranted based on the following observations:

1. All project files (.csproj) consistently show the updated version 3.5.0.
2. Nearly all .cs files in the project have been modified within the last 7 days, indicating significant recent changes or updates to the codebase.
3. The project has a substantial public API surface, as evidenced by the numerous public classes, interfaces, and enums.

These extensive modifications suggest that new features or improvements have likely been added, which aligns with semantic versioning practices for a minor version increase. However, it's crucial to ensure that these changes maintain backwards compatibility.

Recommendation: Conduct a thorough review of the recent changes to confirm they are backwards-compatible and properly documented.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check version consistency and look for changes justifying the minor version bump

# Check for version strings in all .csproj files
echo "Checking version consistency in .csproj files:"
fd -e csproj -x grep -H '<Version>' {}

# Look for new public methods or classes added since the last tag
echo "\nChecking for new public methods or classes:"
git diff $(git describe --tags --abbrev=0)..HEAD | grep -E '^\+.*public.*'

# Check for changes in .cs files since the last tag
echo "\nChecking for changes in .cs files:"
git diff --name-only $(git describe --tags --abbrev=0)..HEAD | grep '\.cs$'

Length of output: 834

---

Script:

#!/bin/bash
# Description: Analyze the codebase structure and recent changes

# List all .cs files in the project
echo "Listing all .cs files in the project:"
fd -e cs

# Check for recently modified .cs files (in the last 7 days)
echo -e "\nRecently modified .cs files (last 7 days):"
find . -name "*.cs" -type f -mtime -7

# Search for public classes and methods in .cs files
echo -e "\nPublic classes and methods in the codebase:"
rg "public (class|interface|struct|enum|record|delegate|static.*method)" -t cs --no-filename | sort | uniq

Length of output: 21250

Nhl.Api/Nhl.Api.csproj (1)

4-4: Version update looks good.

The version has been updated from 3.4.2 to 3.5.0, which aligns with the PR title. This minor version increment suggests new features have been added in a backwards-compatible manner.

Please ensure that:

1. The CHANGELOG.md file has been updated to reflect the changes in this version.
2. The version increment (minor version bump) accurately represents the scope of changes in this release.

You can verify this by running the following script:

Verification successful

---

Version update is justified by new features.

The increase from 3.4.2 to 3.5.0 is appropriate given the addition of new methods:

• GetRealtimePlayerStatisticsBySeasonAndFilterExpressionAsync
• GetTimeOnIcePlayerStatisticsBySeasonAndFilterExpressionAsync

These additions provide enhanced functionality for retrieving player statistics, which aligns with a minor version increment according to semantic versioning principles.

While there's no CHANGELOG.md, the README.md updates effectively document these changes. However, for future updates, consider creating a separate CHANGELOG.md file to track version changes more explicitly.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if CHANGELOG.md exists and contains the new version
if [ -f CHANGELOG.md ]; then
  if grep -q "## \[3.5.0\]" CHANGELOG.md; then
    echo "CHANGELOG.md contains an entry for version 3.5.0"
  else
    echo "Warning: CHANGELOG.md does not contain an entry for version 3.5.0"
  fi
else
  echo "Warning: CHANGELOG.md file not found"
fi

# List all files changed in this PR to help verify the scope of changes
echo "Files changed in this PR:"
git diff --name-only HEAD^

Length of output: 218

---

Script:

#!/bin/bash
# Check the contents of the README.md file for version information
echo "Checking README.md for version 3.5.0 information:"
grep -n -A 5 -B 5 "3\.5\.0" Nhl.Api/README.md

echo "\nDiff of README.md changes:"
git diff HEAD^ Nhl.Api/README.md

Length of output: 29648

Nhl.Api.Domain/Enumerations/Team/TeamEnum.cs (2)

135-135: LGTM: Comma added correctly

The addition of the comma after SeattleKraken = 55 is correct and necessary for extending the enum. This change maintains the existing syntax and allows for the new entry to be added.

---

136-139: Verify team name and enum value

The addition of the UtahHockeyClub entry follows the existing pattern and structure of the enum. However, please verify the following:

1. Is "Utah Hockey Club" the official name of the team? Most NHL team names include both a city and a mascot (e.g., "Seattle Kraken").
2. The enum value 59 is not consecutive with the previous entry (55). Is this intentional?

Let's check if the value 59 is already used elsewhere in the codebase:

Nhl.Api.Domain/Enumerations/Franchise/FranchiseEnum.cs (1)

163-167: LGTM with a minor suggestion.

The addition of the UtahHockeyClub franchise is correct and consistent with the existing enum structure. The value (40) follows the sequence, and the summary comment is provided as expected.

However, I have a small suggestion:

Consider using a more specific name for the franchise if available. "Utah Hockey Club" seems generic and might change once an official team name is announced. If a more specific name is not yet available, it might be worth adding a TODO comment to update it in the future.

Example:

/// <summary>
/// Utah Hockey Club
/// </summary>
/// TODO: Update with official team name when announced
UtahHockeyClub = 40

To ensure this addition doesn't conflict with any existing code or documentation, let's verify its uniqueness:

Verification successful

To further investigate the discrepancy between enum values in FranchiseEnum.cs and TeamEnum.cs, let's run an additional script to compare these files:

---

The UtahHockeyClub entry is consistent with the codebase structure. No changes needed.

After a thorough investigation, I can confirm that the addition of UtahHockeyClub to FranchiseEnum.cs is correct and consistent with the existing codebase structure. Here's a breakdown of the findings:

1. The UtahHockeyClub entry is present in both FranchiseEnum.cs (value 40) and TeamEnum.cs (value 59).

2. The difference in enum values between these files is intentional and consistent with the overall structure of each enum:

   • FranchiseEnum appears to list all historical and current NHL franchises, with values ranging from 1 to 40.
   • TeamEnum likely represents current active teams, with non-consecutive values ranging from 1 to 59.

3. The name "Utah Hockey Club" is used consistently across multiple files in the codebase, indicating it's an established term.

Given these observations, my earlier suggestion to add a TODO comment for updating the name is unnecessary. The implementation is correct and aligns with the existing code structure.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for any other occurrences of Utah Hockey Club or the value 40
rg -i "utah.*(hockey|club)" --type cs
rg "= 40" --type cs

Length of output: 1260

---

Script:

#!/bin/bash
# Compare enum values in FranchiseEnum.cs and TeamEnum.cs
echo "FranchiseEnum.cs:"
rg "= \d+" Nhl.Api.Domain/Enumerations/Franchise/FranchiseEnum.cs | sort -n -k3
echo "\nTeamEnum.cs:"
rg "= \d+" Nhl.Api.Domain/Enumerations/Team/TeamEnum.cs | sort -n -k3

Length of output: 2124

Nhl.Api.Domain/Models/Statistics/PlayerTimeOnIceStatisticsFilterResult.cs (2)

4-22: LGTM: Well-structured class with clear purpose and proper JSON serialization.

The PlayerTimeOnIceStatisticsFilterResult class is well-implemented with descriptive property names and appropriate use of JsonProperty attributes for serialization. The summary comments provide clear information about the class and its properties.

---

24-154: LGTM: Comprehensive player statistics class with clear property descriptions.

The PlayerTimeOnIceStatisticsResult class is well-implemented with descriptive property names, appropriate use of JsonProperty attributes for serialization, and clear summary comments for each property.

Nhl.Api.Domain/Models/Statistics/GoalieStatisticsFilterResult.cs (4)

94-97: LGTM: Nullable OvertimeLosses property

The change from int to int? for the OvertimeLosses property is appropriate. It allows for scenarios where overtime losses data might not be available, improving the flexibility of the data model. The updated comment accurately reflects this change.

---

122-125: Approved: Improved SavePercentage property

The change from double to decimal? for the SavePercentage property is a good improvement:

1. It allows for scenarios where save percentage data might not be available.
2. Using decimal instead of double is more appropriate for precise decimal calculations, which is crucial for statistics like save percentages.

The updated comment accurately reflects this change.

---

129-132: LGTM: Nullable Saves property

The change from int to int? for the Saves property is appropriate. It allows for scenarios where saves data might not be available, improving the flexibility of the data model. The updated comment accurately reflects this change.

---

150-153: LGTM: Nullable ShotsAgainst property

The change from int to int? for the ShotsAgainst property is appropriate. It allows for scenarios where shots against data might not be available, improving the flexibility of the data model. The updated comment accurately reflects this change.

Nhl.Api.Domain/Models/Statistics/PlayerStatisticsFilterResult.cs (9)

38-41: LGTM: Nullable type for EvenStrengthGoals

The change from int to int? for EvenStrengthGoals is appropriate. It allows for representing missing or undefined data, which is common in sports statistics. The updated XML comment accurately reflects this change.

---

45-48: LGTM: Nullable type for EvenStrengthPoints

The change to int? for EvenStrengthPoints is consistent with the previous property change. It appropriately allows for null values, and the XML comment has been updated accordingly.

---

108-111: LGTM: Nullable type for PlusMinus

Changing PlusMinus to int? is a good decision. Plus/minus can be undefined in certain situations (e.g., goalies), so allowing null values enhances the model's flexibility. The XML comment has been appropriately updated.

---

136-139: LGTM: Nullable type for PowerPlayGoals

The change to int? for PowerPlayGoals is consistent with the previous changes and appropriate for the domain. It allows for representing situations where power play goal data might be missing or undefined. The XML comment has been correctly updated.

---

143-146: LGTM: Nullable type for PowerPlayPoints

Changing PowerPlayPoints to int? is consistent with the PowerPlayGoals change and other properties. This allows for null values when data is missing or undefined, which is appropriate for power play statistics. The XML comment has been properly updated.

---

157-160: LGTM: Nullable type for ShortHandedGoals

The change to int? for ShortHandedGoals is consistent with the previous property changes. It appropriately allows for null values in cases where short-handed goal data might be missing or undefined. The XML comment has been correctly updated to reflect this change.

---

164-167: LGTM: Nullable type for ShortHandedPoints

Changing ShortHandedPoints to int? is consistent with the ShortHandedGoals change and other properties. This allows for null values when data is missing or undefined, which is appropriate for short-handed statistics. The XML comment has been properly updated to reflect the nullable nature of the property.

---

171-174: LGTM: Updated comment for ShootingPercentage

The XML comment for ShootingPercentage has been appropriately updated to indicate that the value can be null. This is consistent with the property's existing nullable type (decimal?) and aligns with the changes made to other properties in this class.

---

Line range hint 1-211: Overall: Improved flexibility for representing player statistics

The changes in this file consistently update several properties in the PlayerStatisticsResult class from non-nullable to nullable integer types. This modification enhances the model's ability to represent missing or undefined data, which is common in sports statistics. The affected properties include various goal and point statistics for different game situations (even strength, power play, short-handed).

These changes improve the robustness of the data model and allow for more accurate representation of player statistics, especially in cases where certain data points might not be available or applicable. The XML comments have been consistently updated to reflect these changes, maintaining good documentation practices.

The only minor suggestion is to remove one of the two empty lines at the end of the file to adhere to common coding standards.

Overall, these changes represent a positive improvement to the PlayerStatisticsResult class.

Nhl.Api.Domain/Models/Statistics/PlayerRealtimeStatisticsFilterResult.cs (2)

4-22: LGTM: Well-structured class with clear purpose and proper JSON serialization.

The PlayerRealtimeStatisticsFilterResult class is well-implemented with appropriate documentation and JSON serialization attributes. It follows C# naming conventions and provides a clear structure for representing filtered NHL player statistics results.

---

1-203: Excellent implementation of NHL player statistics models.

This file demonstrates a high-quality implementation of data models for NHL player statistics. Key strengths include:

1. Clear and consistent use of namespaces and JSON serialization attributes.
2. Comprehensive coverage of player statistics with well-named and documented properties.
3. Appropriate use of nullable types for statistical data.
4. Adherence to C# naming conventions and best practices.

The models provide a solid foundation for working with NHL player statistics in the application, balancing between C# conventions and JSON serialization requirements.

Nhl.Api.Domain/Models/Team/TeamCodes.cs (1)

Line range hint 1-244: Verify usage and update documentation for the new team constant.

While the addition of the UtahHockeyClub constant is correct, it's important to ensure that this new team is properly integrated into the rest of the codebase and documentation.

Please run the following script to check for potential places where the new team constant should be used:

Also, please ensure that any relevant documentation, such as API documentation or developer guides, is updated to include information about the Utah Hockey Club.

Nhl.Api.Domain/Services/NhlTeamService.cs (5)

116-116: LGTM: Utah Hockey Club support added correctly

The new case for the Utah Hockey Club has been added correctly to the GetTeamCodeIdentifierByTeamId method. This change is consistent with the existing pattern and properly extends support for the new team.

---

161-161: LGTM: Utah Hockey Club abbreviation mapping added

The new case for the Utah Hockey Club has been correctly added to the GetTeamCodeIdentifierByTeamAbbreviation method. This change maintains consistency with the existing pattern and properly extends support for the new team's abbreviation.

---

205-205: LGTM: Utah Hockey Club enum mapping added

The new case for the Utah Hockey Club has been properly added to the GetTeamCodeIdentifierByTeamEnumeration method. This change is in line with the existing pattern and correctly extends support for the new team's enumeration.

---

Line range hint 1-267: Verify complete integration of Utah Hockey Club

The changes to add support for the Utah Hockey Club in the NhlTeamService class are consistent and follow the existing patterns. However, to ensure full integration, it would be beneficial to verify if any other parts of the codebase need to be updated.

Please run the following script to check for any other occurrences of team-related code that might need updating:

#!/bin/bash
# Description: Check for other occurrences of team-related code

echo "Checking for other team-related occurrences:"
rg --type csharp "(Team(Enum|Names|Codes)|NHL\s+team)" -g '!NhlTeamService.cs'

echo "Checking for test files that might need updating:"
fd -e cs | rg -i "test"

Review the output to ensure that all necessary updates have been made across the codebase to fully support the Utah Hockey Club.

---

265-265: LGTM: Utah Hockey Club name mapping added

The new case for the Utah Hockey Club has been correctly added to the GetTeamCodeIdentifierByTeamName method. This change maintains consistency with the existing pattern and properly extends support for the new team's name.

Please ensure that the TeamNames and TeamCodes enums have been updated in their respective files to include the Utah Hockey Club. Run the following script to verify:

Verification successful

---

Utah Hockey Club successfully added to TeamNames and TeamCodes

The verification process has confirmed that the Utah Hockey Club has been correctly added to both the TeamNames and TeamCodes classes. Specifically:

• In Nhl.Api.Common/Helpers/TeamNames.cs:
  public const string UtahHockeyClub = "Utah Hockey Club";
• In Nhl.Api.Domain/Models/Team/TeamCodes.cs:
  public const string UtahHockeyClub = "UTA";

These additions ensure that the new team is properly represented in the codebase for both its full name and its abbreviated code.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that TeamNames and TeamCodes enums include Utah Hockey Club

# Check TeamNames enum
echo "Checking TeamNames enum:"
rg --type csharp "public\s+enum\s+TeamNames" -A 50 | rg "UtahHockeyClub"

# Check TeamCodes enum
echo "Checking TeamCodes enum:"
rg --type csharp "public\s+enum\s+TeamCodes" -A 50 | rg "UtahHockeyClub"

Length of output: 105490

---

Script:

#!/bin/bash

# Find files containing TeamNames and TeamCodes enums
echo "Searching for files containing TeamNames and TeamCodes enums:"
fd -e cs | xargs rg "public\s+(enum|static\s+class)\s+(TeamNames|TeamCodes)"

# Search for UtahHockeyClub in the found files
echo "\nSearching for UtahHockeyClub in the enum files:"
fd -e cs | xargs rg "public\s+(enum|static\s+class)\s+(TeamNames|TeamCodes)" -l | xargs rg "UtahHockeyClub"

Length of output: 749

Nhl.Api/Src/StatisticsApi/INhlStatisticsApi.cs (1)

141-163: Overall, good additions to the API.

The two new methods, GetRealtimePlayerStatisticsBySeasonAndFilterExpressionAsync and GetTimeOnIcePlayerStatisticsBySeasonAndFilterExpressionAsync, are well-structured and consistent with the existing interface. They enhance the API's functionality by providing more specific statistical data. The minor suggestions for improvement (default sort value and documentation fix) will further refine these additions.

Nhl.Api/Src/LeagueApi/NhlLeagueApi.cs (1)

413-418: Verify the impact of adding the new team.

While the addition of the Utah Hockey Club colors is correct, it's important to ensure that this change is reflected consistently across the codebase.

Please run the following script to check for any other occurrences of TeamEnum that might need updating:

This will help identify any other places in the code that might need to be updated to accommodate the new team.

Nhl.Api/README.md (1)

Line range hint 1-3010: Summary of README changes

The additions to the README file for the Nhl.Api project are well-documented and consistent with the existing style. Two new methods have been added:

1. GetRealtimePlayerStatisticsBySeasonAndFilterExpressionAsync
2. GetTimeOnIcePlayerStatisticsBySeasonAndFilterExpressionAsync

Both methods are properly integrated into the existing documentation structure, providing clear information about their parameters, return values, and functionality.

The changes to the README file are appropriate and maintain the overall quality of the documentation. The new methods enhance the API's capabilities for retrieving real-time and time-on-ice statistics, which should be valuable for users of the library.

Tools

Markdownlint

1012-1012: Expected: h4; Actual: h5
Heading levels should only increment by one level at a time

(MD001, heading-increment)

---

1024-1024: null
Link fragments should be valid

(MD051, link-fragments)

---

1025-1025: null
Link fragments should be valid

(MD051, link-fragments)

---

1025-1025: null
Link fragments should be valid

(MD051, link-fragments)

---

1026-1026: null
Link fragments should be valid

(MD051, link-fragments)

---

1026-1026: null
Link fragments should be valid

(MD051, link-fragments)

Nhl.Api.Domain/Models/Player/ExpressionPlayerFilterBuilder.cs (3)

45-53: LGTM!

The addition of the AddFilter(PlayerRealtimeStatisticsFilter playerRealtimeStatisticsFilter) method follows the existing pattern and extends functionality appropriately.

---

55-65: LGTM!

The AddFilter(PlayerTimeOnIceStatisticsFilter playerTimeOnIceStatisticsFilter) method is correctly implemented and consistent with the existing code base.

---

Line range hint 66-75: Improved readability with <code> tags in documentation comments

Using <code> tags around keywords like 'contains' enhances the readability of the documentation. This consistent formatting is beneficial for developers referencing these methods.

Nhl.Api.Tests/StatisticsTests.cs (4)

698-702: Incorrect method call in test

In the test method GetRealtimePlayerStatisticsBySeasonAndFilterAsync_Returns_Valid_Results_Based_On_Empty_Expression, the method GetPlayerStatisticsBySeasonAndFilterExpressionAsync is called instead of GetRealtimePlayerStatisticsBySeasonAndFilterExpressionAsync. This mismatch can lead to incorrect test coverage.

Apply this diff to correct the method call:

-            var result = await nhlApi.GetPlayerStatisticsBySeasonAndFilterExpressionAsync(SeasonYear.season20102011, expression);
+            var result = await nhlApi.GetRealtimePlayerStatisticsBySeasonAndFilterExpressionAsync(SeasonYear.season20102011, expression);

Likely invalid or redundant comment.

---

699-699: Change comment to '// Assert'

The comment on line 699 should be '// Assert' instead of '// Act' since the code is performing assertions.

Apply this diff to correct the comment:

-            // Act
+            // Assert

Likely invalid or redundant comment.

---

739-739: Change comment to '// Assert'

The comment on line 739 should be '// Assert' instead of '// Act' since the code is performing assertions.

Apply this diff to correct the comment:

-            // Act
+            // Assert

Likely invalid or redundant comment.

---

793-793: Change comment to '// Assert'

The comment on line 793 should be '// Assert' instead of '// Act' since the code is performing assertions.

Apply this diff to correct the comment:

-            // Act
+            // Assert

Likely invalid or redundant comment.

Nhl.Api/Src/Api/NhlApi.cs (2)

667-668: Implementation of GetRealtimePlayerStatisticsBySeasonAndFilterExpressionAsync is correct

The method correctly delegates the call to _nhlStatisticsApi, passing all parameters appropriately.

---

680-681: Implementation of GetTimeOnIcePlayerStatisticsBySeasonAndFilterExpressionAsync is correct

The method correctly calls _nhlStatisticsApi with the appropriate parameters.

[coderabbitai]

Actionable comments posted: 3

Outside diff range and nitpick comments (5)

Nhl.Api/Src/GlobalUsings.cs (1)

14-14: Approved. Consider further organizing global using directives.

Moving the Nhl.Api.Models.Enumerations.Player namespace is fine and doesn't affect functionality. It's now grouped more logically with other project-specific namespaces.

As a suggestion for future improvements, consider organizing all global using directives into logical groups (e.g., System namespaces, third-party libraries, and project-specific namespaces) and sorting them alphabetically within each group for better readability and maintenance.

Nhl.Api.Domain/Services/NhlTeamService.cs (2)

20-20: LGTM! Consider updating documentation for nullable returns.

The change to nullable string return types (string?) for these methods improves flexibility and aligns with C# nullable reference types. This is a good practice for handling potential null values.

Consider updating the method documentation to reflect that these methods can now return null, and under what circumstances a null value might be returned.

Also applies to: 27-27, 34-34, 41-41, 55-55

---

Based on the script output, we can confirm the inconsistency in the GetTeamCodeIdentifierByTeamName method. Let's generate the final response.

Inconsistent return type in GetTeamCodeIdentifierByTeamName method

The GetTeamCodeIdentifierByTeamName method in the NhlTeamService class is inconsistent with the other methods and its interface declaration. It currently returns string instead of string?.

To fix this:

1. Change the method signature in the NhlTeamService class from:

   public string GetTeamCodeIdentifierByTeamName(string teamName)

   to:

   public string? GetTeamCodeIdentifierByTeamName(string teamName)

2. Consider handling the ArgumentException case to return null instead of throwing an exception, to be consistent with the nullable return type and the behavior of other methods.

This change will ensure consistency across the service and prevent potential null reference exceptions for consumers of the API.

Analysis chain

Line range hint 1-268: Overall good changes with one inconsistency to address.

The changes in this file generally improve the API by allowing for nullable returns, which provides more flexibility in handling unknown or invalid inputs. The addition of the Utah Hockey Club is consistent across all methods, which is good for maintainability.

Key points:

1. The interface and most method implementations now correctly use string? return types.
2. The Utah Hockey Club has been added consistently to all relevant switch statements.
3. There's an inconsistency in the GetTeamCodeIdentifierByTeamName method that should be addressed as per the previous comment.

These changes may introduce breaking changes for consumers of this API, so ensure that this is communicated in release notes and version numbers are updated appropriately.

To ensure all methods are consistent, run the following script:

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all public methods in NhlTeamService.cs return string?

# Test: Check if all public methods in the interface and class return string?
rg --type csharp 'public string\?? \w+\(' Nhl.Api.Domain/Services/NhlTeamService.cs

Length of output: 947

Nhl.Api.Tests/StatisticsTests.cs (2)

619-653: LGTM with a minor suggestion

This new test method effectively checks the retrieval of real-time player statistics with specific filters. It's well-structured and covers an important use case.

There's a minor issue with a comment:

-        // Act
+        // Assert

This change aligns the comment with the actual operation being performed in that section of the test.

---

655-684: LGTM with a minor suggestion

This new test method effectively checks the retrieval of real-time player statistics for the 2020-2021 season with specific filters. It's well-structured and provides good coverage for a particular use case.

There's a minor issue with a comment:

-        // Act
+        // Assert

This change aligns the comment with the actual operation being performed in that section of the test.

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between bcc9d49 and 283301c.

Files selected for processing (50)

• Nhl.Api.Common/Http/NhlApiHttpClient.cs (1 hunks)
• Nhl.Api.Common/Services/CachingService.cs (1 hunks)
• Nhl.Api.Domain/Enumerations/Player/PlayerEnumFileGeneratorHelper.cs (1 hunks)
• Nhl.Api.Domain/Models/Game/GameCenterBoxScore.cs (1 hunks)
• Nhl.Api.Domain/Models/Game/GameCenterLanding.cs (1 hunks)
• Nhl.Api.Domain/Models/Game/GameCenterPlayByPlay.cs (1 hunks)
• Nhl.Api.Domain/Models/Game/GameMetadata.cs (1 hunks)
• Nhl.Api.Domain/Models/Game/GameScore.cs (1 hunks)
• Nhl.Api.Domain/Models/Game/GameScoreboard.cs (1 hunks)
• Nhl.Api.Domain/Models/Game/GoalieSeasonGameLog.cs (1 hunks)
• Nhl.Api.Domain/Models/Game/PlayerSeasonGameLog.cs (1 hunks)
• Nhl.Api.Domain/Models/Game/TeamScoreboard.cs (1 hunks)
• Nhl.Api.Domain/Models/Game/TvScheduleBroadcast.cs (1 hunks)
• Nhl.Api.Domain/Models/League/LeagueMetadataInformation.cs (1 hunks)
• Nhl.Api.Domain/Models/Player/ExpressionGoalieFilterBuilder.cs (1 hunks)
• Nhl.Api.Domain/Models/Player/ExpressionPlayerFilterBuilder.cs (13 hunks)
• Nhl.Api.Domain/Models/Player/GoalieProfile.cs (1 hunks)
• Nhl.Api.Domain/Models/Player/PlayerData.cs (1 hunks)
• Nhl.Api.Domain/Models/Player/PlayerProfile.cs (1 hunks)
• Nhl.Api.Domain/Models/Player/PlayerShiftInformation.cs (1 hunks)
• Nhl.Api.Domain/Models/Player/TeamProspects.cs (1 hunks)
• Nhl.Api.Domain/Models/Schedule/LeagueSchedule.cs (1 hunks)
• Nhl.Api.Domain/Models/Schedule/LeagueScheduleCalendar.cs (1 hunks)
• Nhl.Api.Domain/Models/Schedule/TeamSchedule.cs (1 hunks)
• Nhl.Api.Domain/Models/Schedule/TeamSeasonSchedule.cs (1 hunks)
• Nhl.Api.Domain/Models/Schedule/TeamWeekSchedule.cs (1 hunks)
• Nhl.Api.Domain/Models/Season/LeagueStandingsSeasonInformation.cs (1 hunks)
• Nhl.Api.Domain/Models/Season/TeamStatisticsSeason.cs (1 hunks)
• Nhl.Api.Domain/Models/Standing/LeagueStanding.cs (1 hunks)
• Nhl.Api.Domain/Models/Statistics/GoalieStatisticsFilterResult.cs (4 hunks)
• Nhl.Api.Domain/Models/Statistics/GoalieStatisticsLeaders.cs (1 hunks)
• Nhl.Api.Domain/Models/Statistics/PlayerRealtimeStatisticsFilterResult.cs (1 hunks)
• Nhl.Api.Domain/Models/Statistics/PlayerStatisticLeaders.cs (1 hunks)
• Nhl.Api.Domain/Models/Statistics/PlayerStatisticsFilterResult.cs (6 hunks)
• Nhl.Api.Domain/Models/Statistics/PlayerTimeOnIceStatisticsFilterResult.cs (1 hunks)
• Nhl.Api.Domain/Models/Team/TeamSeasonRoster.cs (1 hunks)
• Nhl.Api.Domain/Models/Team/TeamSeasonStatistics.cs (1 hunks)
• Nhl.Api.Domain/Services/NhlTeamService.cs (11 hunks)
• Nhl.Api.Tests/GameTests.cs (1 hunks)
• Nhl.Api.Tests/GlobalUsings.cs (1 hunks)
• Nhl.Api.Tests/LeagueTests.cs (2 hunks)
• Nhl.Api.Tests/NhlApiAsyncHelperTests.cs (1 hunks)
• Nhl.Api.Tests/PlayerTests.cs (1 hunks)
• Nhl.Api.Tests/ProjectStructureTests.cs (2 hunks)
• Nhl.Api.Tests/StatisticsTests.cs (8 hunks)
• Nhl.Api.Tests/TeamTests.cs (2 hunks)
• Nhl.Api/Src/Api/NhlApi.cs (1 hunks)
• Nhl.Api/Src/GlobalUsings.cs (1 hunks)
• Nhl.Api/Src/StatisticsApi/INhlStatisticsApi.cs (2 hunks)
• Nhl.Api/Src/StatisticsApi/NhlStatisticsApi.cs (2 hunks)

Files skipped from review due to trivial changes (36)

• Nhl.Api.Common/Http/NhlApiHttpClient.cs
• Nhl.Api.Common/Services/CachingService.cs
• Nhl.Api.Domain/Enumerations/Player/PlayerEnumFileGeneratorHelper.cs
• Nhl.Api.Domain/Models/Game/GameCenterBoxScore.cs
• Nhl.Api.Domain/Models/Game/GameCenterLanding.cs
• Nhl.Api.Domain/Models/Game/GameCenterPlayByPlay.cs
• Nhl.Api.Domain/Models/Game/GameMetadata.cs
• Nhl.Api.Domain/Models/Game/GameScore.cs
• Nhl.Api.Domain/Models/Game/GameScoreboard.cs
• Nhl.Api.Domain/Models/Game/GoalieSeasonGameLog.cs
• Nhl.Api.Domain/Models/Game/PlayerSeasonGameLog.cs
• Nhl.Api.Domain/Models/Game/TeamScoreboard.cs
• Nhl.Api.Domain/Models/Game/TvScheduleBroadcast.cs
• Nhl.Api.Domain/Models/League/LeagueMetadataInformation.cs
• Nhl.Api.Domain/Models/Player/ExpressionGoalieFilterBuilder.cs
• Nhl.Api.Domain/Models/Player/GoalieProfile.cs
• Nhl.Api.Domain/Models/Player/PlayerData.cs
• Nhl.Api.Domain/Models/Player/PlayerProfile.cs
• Nhl.Api.Domain/Models/Player/PlayerShiftInformation.cs
• Nhl.Api.Domain/Models/Player/TeamProspects.cs
• Nhl.Api.Domain/Models/Schedule/LeagueSchedule.cs
• Nhl.Api.Domain/Models/Schedule/LeagueScheduleCalendar.cs
• Nhl.Api.Domain/Models/Schedule/TeamSchedule.cs
• Nhl.Api.Domain/Models/Schedule/TeamSeasonSchedule.cs
• Nhl.Api.Domain/Models/Schedule/TeamWeekSchedule.cs
• Nhl.Api.Domain/Models/Season/LeagueStandingsSeasonInformation.cs
• Nhl.Api.Domain/Models/Season/TeamStatisticsSeason.cs
• Nhl.Api.Domain/Models/Standing/LeagueStanding.cs
• Nhl.Api.Domain/Models/Statistics/GoalieStatisticsLeaders.cs
• Nhl.Api.Domain/Models/Statistics/PlayerStatisticLeaders.cs
• Nhl.Api.Domain/Models/Team/TeamSeasonRoster.cs
• Nhl.Api.Domain/Models/Team/TeamSeasonStatistics.cs
• Nhl.Api.Tests/GameTests.cs
• Nhl.Api.Tests/GlobalUsings.cs
• Nhl.Api.Tests/NhlApiAsyncHelperTests.cs
• Nhl.Api.Tests/PlayerTests.cs

Files skipped from review as they are similar to previous changes (11)

• Nhl.Api.Domain/Models/Player/ExpressionPlayerFilterBuilder.cs
• Nhl.Api.Domain/Models/Statistics/GoalieStatisticsFilterResult.cs
• Nhl.Api.Domain/Models/Statistics/PlayerRealtimeStatisticsFilterResult.cs
• Nhl.Api.Domain/Models/Statistics/PlayerStatisticsFilterResult.cs
• Nhl.Api.Domain/Models/Statistics/PlayerTimeOnIceStatisticsFilterResult.cs
• Nhl.Api.Tests/LeagueTests.cs
• Nhl.Api.Tests/ProjectStructureTests.cs
• Nhl.Api.Tests/TeamTests.cs
• Nhl.Api/Src/Api/NhlApi.cs
• Nhl.Api/Src/StatisticsApi/INhlStatisticsApi.cs
• Nhl.Api/Src/StatisticsApi/NhlStatisticsApi.cs

Additional comments not posted (8)

Nhl.Api/Src/GlobalUsings.cs (1)

3-8: LGTM! Good addition of global using directives.

The addition of these common System namespace global using directives is a positive change. It will simplify code throughout the project by reducing the need for explicit using statements in individual files. This aligns well with modern C# practices, especially since the introduction of global using directives in C# 10.

Nhl.Api.Domain/Services/NhlTeamService.cs (4)

70-70: LGTM! Implementation correctly handles nullable return.

The change to string? return type is consistent with the interface update. The implementation correctly handles the possibility of returning null for unknown team logo types.

---

82-82: LGTM! New team added and nullable return handled correctly.

The change to string? return type is consistent with the interface update. The new case for Utah Hockey Club is correctly implemented, and the default case returning null is appropriate for unknown team IDs.

Also applies to: 116-116

---

125-125: LGTM! New team added and nullable return handled correctly.

The change to string? return type is consistent with the interface update. The new case for Utah Hockey Club is correctly implemented, and the default case returning null is appropriate for unknown team abbreviations.

Also applies to: 161-161

---

171-171: LGTM! New team added and nullable return handled correctly.

The change to string? return type is consistent with the interface update. The new case for Utah Hockey Club is correctly implemented, and the default case returning null is appropriate for unknown team enumerations.

Also applies to: 205-205

Nhl.Api.Tests/StatisticsTests.cs (3)

755-808: LGTM! Comprehensive test for complex query

This test method effectively checks the retrieval of real-time player statistics with a complex filter query, including nested conditions. It's well-structured and provides excellent coverage for intricate query scenarios.

The assertions thoroughly validate all the filter conditions, ensuring that the API correctly handles complex combinations of filters. This is crucial for maintaining the reliability and flexibility of the statistics retrieval functionality.

---

Line range hint 854-1461: Excellent expansion of test coverage

The additions to this test file significantly enhance the coverage of the NHL API's statistics-related functionality. Here are the key improvements:

1. Comprehensive season coverage: The data-driven tests (lines 854-961 and 975-1082) ensure that the API functions correctly across a wide range of seasons, which is crucial for historical data integrity.

2. Edge case handling: Tests for the 2004-2005 lockout season (lines 1097-1128) verify that the API correctly handles this exceptional case.

3. Error case testing: The new tests for invalid inputs (e.g., lines 1249-1461) are valuable for ensuring proper input validation and error handling in the API.

4. Complex query testing: The tests for complex filter queries (e.g., lines 1140-1244) verify that the API can handle intricate combinations of filters correctly.

These additions significantly strengthen the test suite, providing better assurance of the API's reliability and correctness across various scenarios and edge cases.

---

Line range hint 1-1461: Overall assessment: Significant improvement in test coverage

This update to the StatisticsTests.cs file represents a substantial enhancement to the test suite for the NHL API's statistics functionality. The key strengths of these changes include:

1. Expanded coverage of complex queries and filter combinations.
2. Comprehensive testing across multiple NHL seasons.
3. Improved error case and edge condition testing.
4. Use of data-driven tests to efficiently cover multiple scenarios.

While there are a few minor issues with comments and method names, these do not detract from the overall quality and value of the additions. Addressing these small inconsistencies will further improve the clarity and maintainability of the test suite.

These changes significantly strengthen the reliability and robustness of the NHL API by ensuring its correct behavior across a wide range of scenarios and input conditions.

[coderabbitai]

Actionable comments posted: 0

Outside diff range and nitpick comments (3)

Nhl.Api/README.md (3)

161-163: LGTM! Minor formatting suggestion.

The new method GetRealtimePlayerStatisticsBySeasonAndFilterExpressionAsync is well-documented and consistent with the existing API style. However, for better readability, consider breaking the long parameter list into multiple lines, aligning each parameter.

Consider reformatting the parameter list like this:

GetRealtimePlayerStatisticsBySeasonAndFilterExpressionAsync(
    seasonYear,
    expressionPlayerFilter,
    playerRealtimeStatisticsFilterToSortBy,
    limit,
    offsetStart,
    cancellationToken)

---

184-186: LGTM! Minor formatting suggestion.

The new method GetTimeOnIcePlayerStatisticsBySeasonAndFilterExpressionAsync is well-documented and consistent with the existing API style. However, for better readability, consider breaking the long parameter list into multiple lines, aligning each parameter.

Consider reformatting the parameter list like this:

GetTimeOnIcePlayerStatisticsBySeasonAndFilterExpressionAsync(
    seasonYear,
    expressionPlayerFilter,
    playerTimeOnIceStatisticsFilterToSortBy,
    limit,
    offsetStart,
    cancellationToken)

---

Line range hint 1-3070: Minor markdown structure improvements needed

The README file is comprehensive and well-organized. However, there are a few minor markdown structure issues that should be addressed:

1. Some heading levels are inconsistent. Ensure that heading levels only increment by one level at a time.
2. There are invalid link fragments in some of the documentation. Please review and correct these links to ensure they point to the correct sections.

Addressing these issues will improve the overall structure and navigation of the documentation.

Tools

Markdownlint

1012-1012: Expected: h4; Actual: h5
Heading levels should only increment by one level at a time

(MD001, heading-increment)

---

1024-1024: null
Link fragments should be valid

(MD051, link-fragments)

---

1025-1025: null
Link fragments should be valid

(MD051, link-fragments)

---

1025-1025: null
Link fragments should be valid

(MD051, link-fragments)

---

1026-1026: null
Link fragments should be valid

(MD051, link-fragments)

---

1026-1026: null
Link fragments should be valid

(MD051, link-fragments)

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 283301c and 67a50d0.

Files selected for processing (1)

• Nhl.Api/README.md (7 hunks)

Additional context used

Markdownlint

Nhl.Api/README.md

1012-1012: Expected: h4; Actual: h5
Heading levels should only increment by one level at a time

(MD001, heading-increment)

---

1024-1024: null
Link fragments should be valid

(MD051, link-fragments)

---

1025-1025: null
Link fragments should be valid

(MD051, link-fragments)

---

1025-1025: null
Link fragments should be valid

(MD051, link-fragments)

---

1026-1026: null
Link fragments should be valid

(MD051, link-fragments)

---

1026-1026: null
Link fragments should be valid

(MD051, link-fragments)

---

1450-1450: Expected: h4; Actual: h5
Heading levels should only increment by one level at a time

(MD001, heading-increment)

---

1462-1462: null
Link fragments should be valid

(MD051, link-fragments)

---

1463-1463: null
Link fragments should be valid

(MD051, link-fragments)

---

1463-1463: null
Link fragments should be valid

(MD051, link-fragments)

---

1464-1464: null
Link fragments should be valid

(MD051, link-fragments)

---

1464-1464: null
Link fragments should be valid

(MD051, link-fragments)

---

2873-2873: Expected: h4; Actual: h5
Heading levels should only increment by one level at a time

(MD001, heading-increment)

---

2885-2885: null
Link fragments should be valid

(MD051, link-fragments)

---

2886-2886: null
Link fragments should be valid

(MD051, link-fragments)

---

2886-2886: null
Link fragments should be valid

(MD051, link-fragments)

---

2887-2887: null
Link fragments should be valid

(MD051, link-fragments)

---

2887-2887: null
Link fragments should be valid

(MD051, link-fragments)

---

2992-2992: Expected: h4; Actual: h5
Heading levels should only increment by one level at a time

(MD001, heading-increment)

---

3004-3004: null
Link fragments should be valid

(MD051, link-fragments)

---

3005-3005: null
Link fragments should be valid

(MD051, link-fragments)

---

3005-3005: null
Link fragments should be valid

(MD051, link-fragments)

---

3006-3006: null
Link fragments should be valid

(MD051, link-fragments)

---

3006-3006: null
Link fragments should be valid

(MD051, link-fragments)

[coderabbitai]

Actionable comments posted: 2

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 67a50d0 and 8c1f8a4.

Files selected for processing (3)

• Nhl.Api.Domain/Services/NhlTeamService.cs (11 hunks)
• Nhl.Api.Tests/Helpers/Attributes/TestMethodWithRetryAttribute.cs (2 hunks)
• Nhl.Api.Tests/TeamTests.cs (1 hunks)

Files skipped from review as they are similar to previous changes (2)

• Nhl.Api.Domain/Services/NhlTeamService.cs
• Nhl.Api.Tests/TeamTests.cs

Additional comments not posted (2)

Nhl.Api.Tests/Helpers/Attributes/TestMethodWithRetryAttribute.cs (2)

1-1: Proper Inclusion of System.Linq Namespace

The addition of using System.Linq; is appropriate and necessary for utilizing LINQ methods like Any() and First() later in the code.

---

19-19: Changed Default Value for RetryDelayInSeconds

The default value of RetryDelayInSeconds has been updated from 0 to 1. This introduces a minimum delay of one second between retries. Ensure that this change aligns with the desired testing behavior and does not negatively impact tests that require immediate retries.

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between d347fd8 and 7ebac93.

📒 Files selected for processing (1)

• Nhl.Api/Src/GameApi/NhlGameApi.cs (1 hunks)

🔇 Additional comments (1)

Nhl.Api/Src/GameApi/NhlGameApi.cs (1)

156-167: Efficient asynchronous data retrieval implemented correctly

The refactored GetGameCenterBoxScoreByGameIdAsync method improves efficiency by fetching data concurrently using Task.WhenAll and combining the results. The implementation aligns with asynchronous programming best practices.

[coderabbitai]

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (2)

Nhl.Api.Tests/StatisticsTests.cs (2)

1097-1111: Inconsistent use of test method attributes

The test method GetPlayerStatisticsBySeasonAndFilterExpressionAsync_Returns_Valid_Results_For_Each_Season_With_04_05_Lockout is marked with [TestMethod], whereas other test methods in this class use [TestMethodWithRetry(RetryCount = 5)]. For consistency, consider updating the attribute to include the retry logic.

---

1113-1128: Inconsistent use of test method attributes

The test method GetGoalieStatisticsBySeasonAndFilterAsync_Returns_Valid_Results_For_Each_Season_With_04_05_Lockout is marked with [TestMethod], while other test methods use [TestMethodWithRetry(RetryCount = 5)]. To maintain consistency and potentially improve test reliability, consider applying the retry attribute.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 7ebac93 and e9a494b.

📒 Files selected for processing (1)

• Nhl.Api.Tests/StatisticsTests.cs (15 hunks)

🔇 Additional comments (1)

Nhl.Api.Tests/StatisticsTests.cs (1)

686-702: Method name inconsistent with API call [Duplicate Issue]

This issue was previously identified, and the past review comment still applies.