Manage user stats in score board #146
Conversation
WalkthroughThe pull request includes updates to several files, primarily focusing on restructuring user statistics models in a sports context. Key changes involve the removal of outdated classes and the introduction of new classes for batting, bowling, and fielding statistics. Additionally, new methods for calculating and managing user statistics are implemented, along with modifications to the user service for fetching statistics. The changes also include updates to UI components and view models to reflect the new data structures, enhancing the overall management of user performance metrics. Changes
Possibly related PRs
Suggested reviewers
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? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
cp-mayank
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Outside diff range and nitpick comments (33)
khelo/lib/ui/flow/team/user_detail/component/user_detail_fielding_content.dart (1)
Line range hint
21-62: LGTM! Consider adding documentation for the statsDataRow widget.The build method implementation is clean and well-structured. It properly handles null values and uses localization consistently.
Consider adding documentation for the
statsDataRowwidget to explain its parameters and usage, as it seems to be a reusable component.data/lib/utils/constant/firestore_constant.dart (1)
11-11: Consider using plural form for collection nameFor consistency with other collection names (
matchesCollection,teamsCollection,usersCollection), consider using plural form:- static const String userStatCollection = "user_stat"; + static const String userStatsCollection = "user_stats";khelo/lib/ui/flow/team/user_detail/component/user_detail_batting_content.dart (1)
Line range hint
1-5: Remove unused importThe import of
user_detail_bowling_content.dartappears to be unused in this file.import 'package:data/api/user/user_models.dart'; import 'package:flutter/material.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:khelo/domain/extensions/context_extensions.dart'; -import 'package:khelo/ui/flow/team/user_detail/component/user_detail_bowling_content.dart';khelo/lib/ui/flow/team/user_detail/component/user_detail_bowling_content.dart (1)
Line range hint
117-170: Consider extracting the statsDataRow to a shared componentThe statsDataRow implementation is well-structured, but since it's likely used across different stat views (batting, bowling, fielding), consider extracting it to a shared component for better reusability.
+ // TODO: Move to a shared component like 'components/stats_data_row.dart' Widget statsDataRow(...)khelo/lib/ui/flow/stats/user_stat/user_stat_screen.dart (1)
115-120: Consider optimizing stats retrieval and improving error handlingThe current implementation has a few areas for improvement:
- The same iteration pattern is repeated twice
- Silent fallbacks to empty stats might mask data loading issues
Consider refactoring to:
- final testStats = state.userStats - ?.firstWhere((element) => element.type == UserStatType.test) ?? - UserStat(); - final otherStats = state.userStats - ?.firstWhere((element) => element.type == UserStatType.other) ?? - UserStat(); + final stats = state.userStats?.fold<Map<UserStatType, UserStat>>( + {UserStatType.test: UserStat(), UserStatType.other: UserStat()}, + (map, stat) => map..[stat.type] = stat, + ) ?? {UserStatType.test: UserStat(), UserStatType.other: UserStat()}; + + final testStats = stats[UserStatType.test]!; + final otherStats = stats[UserStatType.other]!;This approach:
- Reduces iterations through userStats
- Makes the default values more explicit
- Maintains the same functionality while being more efficient
data/lib/api/tournament/tournament_model.dart (1)
139-140: Consider adding null safety checks for stats properties.While the code assumes the presence of batting and bowling stats, it might be safer to add null checks to handle cases where these stats might not be initialized yet.
- final battingStats = playerStat.stats.batting; - final bowlingStats = playerStat.stats.bowling; + final battingStats = playerStat.stats.batting; + final bowlingStats = playerStat.stats.bowling; + + // Skip if required stats are not available + if (battingStats == null || bowlingStats == null) continue;khelo/lib/ui/flow/team/user_detail/user_detail_screen.dart (1)
170-175: Consider optimizing stats retrieval and null handling.The current implementation has a few potential improvements:
- Using
firstWhereon potentially large lists could be inefficient.- Creating new
UserStatinstances as fallbacks could be wasteful.Consider this alternative approach:
- final testStats = state.userStats - ?.firstWhere((element) => element.type == UserStatType.test) ?? - UserStat(); - final otherStats = state.userStats - ?.firstWhere((element) => element.type == UserStatType.other) ?? - UserStat(); + // Cache the stats list to avoid multiple null checks + final stats = state.userStats; + // Use a shared empty stat instance + static final _emptyStats = UserStat(); + + final testStats = stats?.firstWhere( + (element) => element.type == UserStatType.test, + orElse: () => _emptyStats, + ) ?? _emptyStats; + + final otherStats = stats?.firstWhere( + (element) => element.type == UserStatType.other, + orElse: () => _emptyStats, + ) ?? _emptyStats;khelo/lib/ui/flow/tournament/detail/tabs/tournament_detail_stats_tab.dart (1)
Line range hint
132-141: Consider enhancing statistics display robustnessThe current implementation could benefit from several improvements:
- Add error boundaries to gracefully handle statistics calculation failures
- Extract number formatting logic into a utility function for consistency
- Consider adding loading states while statistics are being calculated
Consider creating a utility class for statistics formatting:
class StatisticsFormatter { static String formatAverage(double? value) { return value?.toStringAsFixed(1) ?? '0.0'; } static String formatRuns(int? value) { return value?.toString() ?? '0'; } }Then use it in the widget:
- final average = keyStat.stats.bowling.average.toStringAsFixed(1); - final runs = keyStat.stats.batting.runScored.toString(); + final average = StatisticsFormatter.formatAverage(keyStat.stats.bowling?.average); + final runs = StatisticsFormatter.formatRuns(keyStat.stats.batting?.runScored);khelo/lib/ui/flow/team/user_detail/user_detail_view_model.freezed.dart (1)
210-219: Consider optimizing the getter implementationWhile the implementation is functionally correct, it could be optimized by reducing the number of checks:
List<UserStat>? get userStats { - final value = _userStats; - if (value == null) return null; - if (_userStats is EqualUnmodifiableListView) return _userStats; + if (_userStats == null) return null; + if (_userStats is EqualUnmodifiableListView) return _userStats; // ignore: implicit_dynamic_type - return EqualUnmodifiableListView(value); + return EqualUnmodifiableListView(_userStats!); }khelo/lib/ui/flow/stats/user_stat/user_stat_view_model.dart (2)
51-55: Correct the class name in the debug print statementThe debug print statement references
UserDetailViewNotifier, but the class isUserStatViewNotifier. Updating the class name in the log will ensure accurate and consistent logging.Apply this diff to correct the class name:
}, onError: (e) { state = state.copyWith(loading: false, error: e); - debugPrint("UserDetailViewNotifier: error while loading data -> $e"); + debugPrint("UserStatViewNotifier: error while loading data -> $e"); });
78-78: InitializeuserStatswith an empty list instead of nullInitializing
userStatsto an empty list ([]) rather thannullcan help avoid potential null reference errors and eliminates the need for null checks elsewhere in the code.Apply this diff to initialize
userStatsto an empty list:- @Default(null) List<UserStat>? userStats, + @Default([]) List<UserStat> userStats,khelo/lib/ui/flow/team/user_detail/user_detail_view_model.dart (1)
85-85: InitializeuserStatswith an empty list instead of nullCurrently,
userStatsis initialized with@Default(null)and is nullable. For consistency and to simplify null checks, consider initializing it with an empty list using@Default([])and making the type non-nullable.Apply this diff to initialize
userStatswith an empty list:- @Default(null) List<UserStat>? userStats, + @Default([]) List<UserStat> userStats,data/lib/service/user/user_service.dart (2)
64-67: Consider adding pagination to themigratemethod to handle large datasetsFetching all users without pagination might lead to performance issues if the number of users is large. Consider implementing pagination to fetch users in batches to prevent potential memory and resource exhaustion.
183-199: Consider consolidatingcreateNewUserand_createUserto avoid duplicationBoth methods serve to create a new user. Consolidating them can adhere to the DRY (Don't Repeat Yourself) principle and maintain a single source of truth for user creation logic.
data/lib/api/user/user_models.dart (4)
244-337: Inconsistent naming of extension: 'UserStatsExtension'The extension is named
UserStatsExtensionbut extends theUserStatclass. For consistency and clarity, it's better to name the extensionUserStatExtensionto match the class it extends.Apply this diff to rename the extension:
-extension UserStatsExtension on UserStat { +extension UserStatExtension on UserStat {
173-192: Missing or incorrect JSON annotations forUserStatclassThe
@JsonSerializableannotation is placed after the@freezeddecorator, which might lead to improper JSON serialization. According to best practices with thefreezedpackage, the@JsonSerializableannotation should be placed directly above the class declaration.Apply this diff to reposition the annotation:
@freezed +@JsonSerializable(anyMap: true, explicitToJson: true) class UserStat with _$UserStat { - @JsonSerializable(anyMap: true, explicitToJson: true) const factory UserStat({ @Default(0) int matches, UserStatType? type, @Default(Batting()) Batting batting, @Default(Bowling()) Bowling bowling, @Default(Fielding()) Fielding fielding, }) = _UserStat;This ensures that the JSON serialization behaves as expected when using
freezed.
194-242: Consider adding validation for statistical fieldsThe statistical fields in
Batting,Bowling, andFieldingclasses are currently not validated for negative values. Since negative statistics (e.g., negative runs, wickets) are not logical in this context, adding validation checks can prevent erroneous data.You might consider adding assertions or validation logic to ensure that these fields remain non-negative.
245-287: Enhance null safety in statistical aggregation methodsIn the aggregation methods
_addBattingStats,_addBowlingStats, and_addFieldingStats, the null checks might be unnecessary since the fields have default values and are non-nullable due to the@Defaultannotation.Simplify the code by removing null checks:
- if (oldBatting == null) return newBatting!; - if (newBatting == null) return oldBatting;Since
Battinginstances are expected to be non-null, you can remove these checks to streamline the code.khelo/lib/ui/flow/score_board/score_board_view_model.dart (10)
14-14: Consider removing unused imports.The import statement for
package:khelo/domain/extensions/context_extensions.darton line 14 seems to be unused in this file. Removing unused imports can help keep the code clean and improve compilation times.Apply this diff to remove the unused import:
- import 'package:khelo/domain/extensions/context_extensions.dart';
29-29: Ensure consistency in provider dependency injection.In the
scoreBoardStateProvider, theBallScoreServiceis injected after theUserService. However, in theScoreBoardViewNotifierconstructor, the parameters are ordered differently. Consider maintaining a consistent order of dependencies to improve readability and reduce confusion.Here's the constructor for reference:
ScoreBoardViewNotifier( this._matchService, this._inningService, this._userService, this._ballScoreService, )Ensure the order of parameters matches in both places.
37-37: Consider making_userServicea late final variable.The
_userServiceis declared as:final UserService _userService;Since it's injected via the constructor and never reassigned, you might consider declaring it as a
late finalvariable for clarity.late final UserService _userService;
49-49: Possible redundant constructor parameters.The constructor of
ScoreBoardViewNotifiertakes_ballScoreServiceafter_userService, but their order differs from the provider declaration. Ensure that the parameters are in the same order as in the provider for consistency.Ensure the constructor parameters match the order of dependencies injected in the provider.
773-794: Handle asynchronous errors when updating player stats.In the
_addPlayerStatsmethod, you are catching all exceptions broadly. While this prevents the app from crashing, it might mask underlying issues. Consider handling specific exceptions to provide more granularity.Additionally, ensure that the UI reflects any errors so users are aware if stats fail to update.
Apply this diff to log errors with more context:
} catch (e) { - debugPrint( - "ScoreBoardViewNotifier: error while adding player stats -> $e"); + debugPrint( + "ScoreBoardViewNotifier: error while adding player stats -> $e"); + // Optionally, update state with error information + state = state.copyWith( + actionError: e, + isActionInProgress: false, + ); }
Line range hint
920-928: Potential logical error in_isTargetAchievedmethod for Test Matches.In the
_isTargetAchievedmethod, when handlingMatchType.testMatch, there's a logical block:if (state.currentInning?.index == 3 || state.currentInning?.index == 4) { //... }Within this block, you check if
secondPlayingTeam.run > firstPlayingTeam.run, butrunmight not reflect the total runs if innings are incomplete.Verify that the run comparison accurately reflects the match state, especially considering draws or follow-on scenarios in Test cricket.
Line range hint
1086-1099: Incorrect use ofbreakinonMatchOptionSelectswitch-case.In the
onMatchOptionSelectmethod, after each case, there is nobreakstatement. In Dart, unlike some other languages, theswitchstatement doesn't requirebreak, but usingreturnor arranging the code appropriately is important.Ensure that each case correctly handles the intended action without falling through unintentionally.
Example:
switch (option) { case MatchOption.changeStriker: _showStrikerSelectionSheet(); break; case MatchOption.pauseScoring: state = state.copyWith(showPauseScoringSheet: DateTime.now()); break; // ... }Consider adding
breakorreturnstatements to prevent unintended fall-through.
Line range hint
1377-1387: Potential issue with overwritingstatein_configureScoringDetails.After setting several properties on
state, you immediately overwritestateagain. This can lead to lost updates.Consider combining the updates into a single
copyWithor ensuring that subsequent updates include all necessary properties.
Line range hint
1510-1528: Redundant check and potential dead code in_isMatchTiedmethod.In the
_isMatchTiedmethod underMatchType.testMatch, the condition:if (state.currentInning?.index == 4 && _isAllOut()) { //... } else { return false; }Since the
ifblock returns a value, theelseis redundant.Simplify the code by removing unnecessary
elsestatements.if (state.currentInning?.index == 4 && _isAllOut()) { //... } return false;
Line range hint
2014-2033: Handling of active players ingetFilteredPlayerList.In the
getFilteredPlayerListmethod, you're filtering players based on their status. Ensure that players who are suspended or injured are correctly excluded from the list when selecting batsmen or bowlers.Verify that the logic accurately reflects the game's rules and that players who should not be selectable are properly filtered out.
data/lib/api/user/user_models.freezed.dart (5)
1141-1150: Rename 'runScored' to 'runsScored' for consistencyIn the
Battingclass, the fieldrunScoredrepresents the total runs scored by a player. For consistency and clarity, consider renaming it torunsScoredto match the plural form used in similar fields likerunsConcededin theBowlingclass.
1145-1145: Rename 'ballFaced' to 'ballsFaced' for grammatical correctnessIn the
Battingclass, the fieldballFacedshould be pluralized toballsFacedto accurately represent that it refers to multiple balls faced by the player.
1485-1495: Pluralize 'wicketTaken' to 'wicketsTaken'In the
Bowlingclass, the fieldwicketTakenrepresents the number of wickets taken by a player. For grammatical correctness and to indicate that it can be multiple wickets, consider renaming it towicketsTaken.
1841-1843: Pluralize 'runOut' to 'runOuts' in the 'Fielding' classIn the
Fieldingclass, the fieldrunOutshould be renamed torunOutsto reflect that it records the number of run-outs achieved by a player.
1840-1842: Consider initializing Fielding stats with default valuesIn the
Fieldingclass, ensure that all statistics are initialized with default values to prevent potential null reference issues when these fields are accessed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (22)
data/.flutter-plugins-dependencies(1 hunks)data/lib/api/ball_score/ball_score_model.dart(7 hunks)data/lib/api/ball_score/ball_score_model.freezed.dart(0 hunks)data/lib/api/ball_score/ball_score_model.g.dart(0 hunks)data/lib/api/tournament/tournament_model.dart(1 hunks)data/lib/api/user/user_models.dart(1 hunks)data/lib/api/user/user_models.freezed.dart(1 hunks)data/lib/api/user/user_models.g.dart(1 hunks)data/lib/service/user/user_service.dart(3 hunks)data/lib/utils/constant/firestore_constant.dart(2 hunks)khelo/lib/ui/flow/score_board/score_board_view_model.dart(4 hunks)khelo/lib/ui/flow/stats/user_stat/user_stat_screen.dart(2 hunks)khelo/lib/ui/flow/stats/user_stat/user_stat_view_model.dart(5 hunks)khelo/lib/ui/flow/stats/user_stat/user_stat_view_model.freezed.dart(11 hunks)khelo/lib/ui/flow/team/user_detail/component/user_detail_batting_content.dart(2 hunks)khelo/lib/ui/flow/team/user_detail/component/user_detail_bowling_content.dart(2 hunks)khelo/lib/ui/flow/team/user_detail/component/user_detail_fielding_content.dart(3 hunks)khelo/lib/ui/flow/team/user_detail/user_detail_screen.dart(3 hunks)khelo/lib/ui/flow/team/user_detail/user_detail_view_model.dart(2 hunks)khelo/lib/ui/flow/team/user_detail/user_detail_view_model.freezed.dart(13 hunks)khelo/lib/ui/flow/tournament/detail/tabs/tournament_detail_stats_tab.dart(1 hunks)khelo/lib/ui/flow/tournament/detail/tournament_detail_view_model.dart(1 hunks)
💤 Files with no reviewable changes (2)
- data/lib/api/ball_score/ball_score_model.freezed.dart
- data/lib/api/ball_score/ball_score_model.g.dart
✅ Files skipped from review due to trivial changes (1)
- data/.flutter-plugins-dependencies
🔇 Additional comments (44)
khelo/lib/ui/flow/team/user_detail/component/user_detail_fielding_content.dart (2)
Line range hint 1-6: LGTM! Imports and class declaration are well structured.
The imports are appropriate and the class correctly extends ConsumerWidget for state management with Riverpod.
Line range hint 10-19: Verify the properties of the new Fielding type.
The change from FieldingStat? to Fielding? looks correct, but we should verify that all required properties (catches, runOut, stumping) are available in the new type.
✅ Verification successful
Type change from FieldingStat to Fielding is safe and maintains all required properties
The new Fielding class in data/lib/api/user/user_models.dart contains all the necessary properties (catches, runOut, stumping) with the same names and types. The usage in user_detail_fielding_content.dart shows these properties are being accessed correctly. The change is verified to be type-compatible.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the structure of the new Fielding class
# Expected: Should find the definition of Fielding class with required properties
ast-grep --pattern 'class Fielding {
$$$
}'
# Also check for any remaining references to the old FieldingStat type
rg "FieldingStat"
Length of output: 569
Script:
#!/bin/bash
# Let's check the Fielding class definition in the data models
rg -A 10 "^class Fielding"
# Also check for any usages of fielding properties
rg "\.(catches|runOut|stumping)"
Length of output: 4934
data/lib/utils/constant/firestore_constant.dart (1)
32-32: Clarify the purpose of the type field
The constant type is quite generic and its purpose is unclear. Since it's placed in the matches field constants section, consider using a more specific name that indicates its intended use (e.g., matchType, gameType).
✅ Verification successful
Let me search for more context about how this field is used.
Let me try a different approach to find more context.
Let me search for the match model to understand how the type field is used.
Based on the search results, I can now provide a final response:
The type field is used for match type classification
The constant type in FirestoreConstant is used to store the match type in the Firestore database. This is evident from the codebase where match types are handled through MatchType enum, as seen in the match service where matches are created with types like MatchType.limitedOvers. The field name is consistent with Firestore's naming conventions and is appropriately placed in the matches field constants section.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for usages of the 'type' field to understand its context
rg -A 3 '"type"' --type dart
Length of output: 329
Script:
#!/bin/bash
# Search for usages of FirestoreConstant.type to understand its usage
rg -A 3 "FirestoreConstant.type" --type dart
# Also search for any matches-related code to understand the context
ast-grep --pattern 'class $_ {
$$$
match$_($$$) {
$$$
}
$$$
}'
Length of output: 121
Script:
#!/bin/bash
# Get more context around the type constant by showing the entire file
cat data/lib/utils/constant/firestore_constant.dart
# Search for any Firestore-related operations that might use this field
rg -A 5 "\.collection|\.document" --type dart
Length of output: 9898
Script:
#!/bin/bash
# Search for MatchModel class definition and its usage of the type field
ast-grep --pattern 'class MatchModel {
$$$
}'
# Also search for any references to type in match-related files
rg -A 3 "type.*=|type:" --type dart data/lib/
Length of output: 44555
khelo/lib/ui/flow/team/user_detail/component/user_detail_batting_content.dart (2)
Line range hint 22-89: LGTM! Well-structured UI implementation
The build method implementation is well-organized with:
- Proper null safety handling
- Consistent decimal formatting for floating-point values
- Good use of internationalization
- Clean separation of test and other match statistics
10-11: Verify type migration completeness
The change from BattingStat? to Batting? type needs verification to ensure all required properties are available in the new Batting class.
✅ Verification successful
Let me modify the script to correctly search for property accesses and class definition.
Let me search for the old BattingStat class to compare its properties with the new Batting class.
Based on the gathered information, I can now provide the final response:
Type migration from BattingStat to Batting is properly implemented
The new Batting class in data/lib/api/user/user_models.dart contains all necessary properties to support the existing usage. The properties accessed in the codebase (matches, batting, bowling, fielding) are properly handled, and the new class includes comprehensive batting statistics (innings, runScored, average, strikeRate, etc.). The migration appears complete with proper data conversion methods in place.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Compare the properties used in this file with the new Batting class definition
# First, collect all the properties accessed on testStats and otherStats
echo "Properties used in this file:"
rg -o 'test(Stats|Stats\?)\.\K\w+' . | sort -u
rg -o 'other(Stats|Stats\?)\.\K\w+' . | sort -u
# Then, check the definition of the new Batting class
echo -e "\nBatting class definition:"
ast-grep --pattern 'class Batting {
$$$
}'
Length of output: 511
Script:
#!/bin/bash
# Search for property accesses on testStats and otherStats
echo "Properties accessed on testStats:"
rg "testStats\.[a-zA-Z]+" --only-matching
echo -e "\nProperties accessed on otherStats:"
rg "otherStats\.[a-zA-Z]+" --only-matching
echo -e "\nBatting class definition and usage:"
# Find Batting class definition
rg "class Batting" -A 20
# Also search for any imports related to Batting
rg "import.*Batting"
Length of output: 3172
Script:
#!/bin/bash
# Search for the old BattingStat class definition
echo "Old BattingStat class definition:"
rg "class BattingStat" -A 20
# Search for any files that might still be using BattingStat
echo -e "\nFiles still using BattingStat:"
rg "BattingStat"
Length of output: 689
khelo/lib/ui/flow/team/user_detail/component/user_detail_bowling_content.dart (2)
Line range hint 23-115: Implementation looks solid and follows best practices!
The widget implementation demonstrates:
- Proper null-safety handling
- Consistent decimal formatting
- Clean UI structure with proper spacing
- Good use of localization
- Reusable row creation logic
11-12: Verify the type change from BowlingStat to Bowling
The type change looks consistent with the refactoring mentioned in the PR objectives. However, let's ensure all usages are updated correctly.
✅ Verification successful
Type change from BowlingStat to Bowling is correctly implemented
The verification shows that:
- The
Bowlingclass is properly defined inuser_models.dartwith all necessary fields - The type change is consistently implemented across the codebase
- The helper method
_addBowlingStatsinuser_models.dartalready uses theBowlingtype - No legacy
BowlingStattype references remain that would conflict with this change
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining references to BowlingStat
# and verify consistent usage of the new Bowling type
# Search for any remaining BowlingStat references
echo "Checking for any remaining BowlingStat references:"
rg "BowlingStat" -l
# Search for the new Bowling type usage pattern
echo "Checking Bowling type usage pattern:"
rg "Bowling\?" -l
Length of output: 427
Script:
#!/bin/bash
# Let's examine the content of these files to understand the type definitions and usage
# Check the model definitions in user_models.dart
echo "=== Examining user_models.dart ==="
rg "class.*Bowling" -A 5 data/lib/api/user/user_models.dart
# Check if BowlingStat is being used as a type or is it being migrated
echo -e "\n=== Checking BowlingStat usage in user_models.dart ==="
rg "BowlingStat" -A 3 data/lib/api/user/user_models.dart
# Check BowlingStat usage in ball_score_model.dart
echo -e "\n=== Checking BowlingStat in ball_score_model.dart ==="
rg "BowlingStat" -A 3 data/lib/api/ball_score_model.dart
# Check how Bowling type is used in the widget file
echo -e "\n=== Examining usage in user_detail_bowling_content.dart ==="
rg "Bowling\?" -A 3 khelo/lib/ui/flow/team/user_detail/component/user_detail_bowling_content.dart
Length of output: 1399
khelo/lib/ui/flow/stats/user_stat/user_stat_screen.dart (2)
128-143: LGTM! Clean and consistent implementation
The parameter passing to content widgets is well-structured and consistent. The code:
- Maintains a clear separation of concerns
- Properly handles both test and other match statistics
- Follows a consistent pattern across batting, bowling, and fielding components
128-143: Verify content widgets compatibility with new stat models
Since this is part of a larger refactor that introduced new stat models, we should verify that the content widgets are updated to handle the new types correctly.
✅ Verification successful
Based on the output, I can now generate the final response as we have found all the necessary information. The content widgets are properly updated to handle the new model types:
Content widgets are correctly using the new stat models
All three content widgets (UserDetailBattingContent, UserDetailBowlingContent, and UserDetailFieldingContent) are properly importing and using the correct model types from package:data/api/user/user_models.dart:
UserDetailBattingContentusesBatting?type for statsUserDetailBowlingContentusesBowling?type for statsUserDetailFieldingContentusesFielding?type for stats
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the content widgets are updated to use the new model types
ast-grep --pattern 'class UserDetailBattingContent {
$$$
final Batting? testStats;
$$$
}'
ast-grep --pattern 'class UserDetailBowlingContent {
$$$
final Bowling? testStats;
$$$
}'
ast-grep --pattern 'class UserDetailFieldingContent {
$$$
final Fielding? testStats;
$$$
}'
Length of output: 295
Script:
#!/bin/bash
# Let's try to find these content widgets first
fd "user_detail.*content\.dart" --type f --exec cat {}
# Also search for any imports of the stat models
rg "import.*user_models" -A 2
Length of output: 30762
data/lib/api/tournament/tournament_model.dart (1)
139-140: LGTM! Verify the property access changes across the codebase.
The refactoring from battingStat/bowlingStat to batting/bowling improves naming consistency. However, we should ensure all references are updated.
✅ Verification successful
Let me search for any other occurrences of these property accesses to ensure we haven't missed any patterns.
Property access changes are consistent across the codebase
The search results confirm that all references to batting and bowling statistics consistently use the new stats.batting and stats.bowling pattern. The changes in tournament_model.dart align with the usage patterns in other files (tournament_detail_view_model.dart and tournament_detail_stats_tab.dart), showing a uniform approach to accessing these properties.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining old property references
rg "battingStat|bowlingStat" --type dart
Length of output: 1356
Script:
#!/bin/bash
# Search for any references to batting/bowling stats in dart files
rg -A 2 "stats\.(batting|bowling)" --type dart
Length of output: 4693
khelo/lib/ui/flow/team/user_detail/user_detail_screen.dart (2)
1-1: LGTM: Import aligns with the new user stats model.
The addition of the user models import supports the refactored user statistics implementation.
187-196: Verify null handling in child components.
The stats are correctly passed to the batting and bowling components. However, we should ensure that these components handle potentially null stats gracefully.
Let's check the implementation of these components:
✅ Verification successful
Let me try a different approach to find these components and their implementations.
Let me check the actual implementation of these components to see how they handle null stats.
Null handling is properly implemented in both components
Both UserDetailBattingContent and UserDetailBowlingContent handle null stats appropriately:
- The stats parameters (
testStatsandotherStats) are properly declared as nullable types (Batting?andBowling?) - When accessing stats properties, the null-safe operator (?) is used consistently (e.g.,
testStats?.innings.toString()) - The
statsDataRowhelper function handles null values by providing a default '0' value:subtitle1 ?? '0' - Match counts have default values of 0 in the constructor parameters
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check how UserDetailBattingContent and UserDetailBowlingContent handle null stats
# Search for null checks in the batting component
echo "Checking UserDetailBattingContent..."
ast-grep --pattern 'class UserDetailBattingContent {
$$$
}'
# Search for null checks in the bowling component
echo "Checking UserDetailBowlingContent..."
ast-grep --pattern 'class UserDetailBowlingContent {
$$$
}'
Length of output: 293
Script:
#!/bin/bash
# Search for files containing UserDetailBattingContent or UserDetailBowlingContent
echo "Searching for component files..."
fd -e dart | xargs rg "UserDetailBattingContent|UserDetailBowlingContent" -B 2 -A 10
# Also search for required parameters in these components
echo "Searching for required parameters..."
rg "@required.*Stats" -A 2
Length of output: 7725
Script:
#!/bin/bash
# Get the full implementation of both components
echo "UserDetailBattingContent implementation:"
cat khelo/lib/ui/flow/team/user_detail/component/user_detail_batting_content.dart
echo -e "\nUserDetailBowlingContent implementation:"
cat khelo/lib/ui/flow/team/user_detail/component/user_detail_bowling_content.dart
Length of output: 8705
khelo/lib/ui/flow/stats/user_stat/user_stat_view_model.freezed.dart (5)
Line range hint 61-73: LGTM! Proper null-safety implementation
The copy method implementation correctly handles the nullable List<UserStat>? while maintaining immutability.
139-157: LGTM! Proper immutability implementation
The implementation correctly ensures immutability of the stats list using EqualUnmodifiableListView while maintaining proper null-safety.
165-165: LGTM! Proper equality implementations
The toString, equals, and hashCode implementations correctly handle the list property using DeepCollectionEquality for proper collection comparison.
Also applies to: 176-177, 186-186
203-203: LGTM! Verify broader refactoring impact
The abstract class declarations are consistent. However, since this is part of a larger refactoring effort replacing multiple stat properties with a consolidated list, we should verify the impact on dependent code.
Also applies to: 211-211
✅ Verification successful
Properties are correctly used after refactoring
The search results show that the old stat properties (testStats, otherStats, etc.) are being used correctly in the UI components. The code extracts these values from the consolidated userStats list by filtering on UserStatType, which aligns with the refactoring:
user_detail_screen.dartanduser_stat_screen.dartproperly extract stats usingfirstWhereonUserStatType- Component files (
user_detail_batting_content.dart,user_detail_bowling_content.dart,user_detail_fielding_content.dart) correctly receive and display the extracted stats
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for potential usage of old stat properties in the codebase
rg -A 3 "testStats|otherStats|testMatchesCount|otherMatchesCount"
Length of output: 21787
21-21: Verify UserStat type import
The consolidation of stats into a List<UserStat>? improves maintainability. However, let's verify the UserStat type import.
✅ Verification successful
UserStat type is properly imported and used
The verification shows that:
UserStatclass is properly defined indata/lib/api/user/user_models.dart- The view model correctly imports it via
import 'package:data/api/user/user_models.dart' - The freezed file is generated from the view model, so it inherits the imports
The consolidation of multiple stat properties into a List<UserStat>? is well-structured and properly typed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if UserStat type is properly imported and defined
rg -l "class UserStat"
Length of output: 175
Script:
#!/bin/bash
# Check imports in the freezed file and the actual model file
echo "=== Checking imports in user_stat_view_model.freezed.dart ==="
rg "import.*UserStat" khelo/lib/ui/flow/stats/user_stat/user_stat_view_model.freezed.dart
echo -e "\n=== Checking UserStat definition ==="
rg "class UserStat" -A 5 data/lib/api/user/user_models.dart khelo/lib/ui/flow/stats/user_stat/user_stat_screen.dart khelo/lib/ui/flow/stats/user_stat/user_stat_view_model.dart
Length of output: 2550
Script:
#!/bin/bash
# Let's verify the import in the actual view model file and check if UserStat is properly imported there
echo "=== Checking imports in the original view model file ==="
rg "import.*UserStat|import.*user_models" khelo/lib/ui/flow/stats/user_stat/user_stat_view_model.dart
echo -e "\n=== Checking if freezed file imports from the view model ==="
rg "import.*user_stat_view_model\.dart" khelo/lib/ui/flow/stats/user_stat/user_stat_view_model.freezed.dart
Length of output: 522
khelo/lib/ui/flow/tournament/detail/tabs/tournament_detail_stats_tab.dart (1)
133-134: Verify null safety for statistics access
The direct access to bowling.average and batting.runScored assumes these properties will always be present. This could lead to runtime errors if the statistics are null.
Let's verify the assumptions about non-null statistics:
Consider adding null safety:
- final average = keyStat.stats.bowling.average.toStringAsFixed(1);
- final runs = keyStat.stats.batting.runScored.toString();
+ final average = keyStat.stats.bowling?.average?.toStringAsFixed(1) ?? '0.0';
+ final runs = keyStat.stats.batting?.runScored?.toString() ?? '0';khelo/lib/ui/flow/team/user_detail/user_detail_view_model.freezed.dart (6)
24-24: LGTM: Field declaration follows Freezed patterns
The new userStats field is correctly declared as a nullable list using Freezed's mixin pattern.
45-45: LGTM: CopyWith implementation handles nullability correctly
The CopyWith interface and implementation properly handle the nullable List<UserStat> with appropriate null safety checks using the freezed parameter.
Also applies to: 70-93
126-126: LGTM: Implementation class follows consistent patterns
The implementation correctly handles the new userStats field, maintaining consistency with other fields in the class.
Also applies to: 150-173
223-223: LGTM: Proper implementation of equality and hashing
The implementation correctly uses DeepCollectionEquality for comparing and hashing the userStats list, ensuring proper value equality.
Also applies to: 237-238, 249-249
268-268: LGTM: Abstract class maintains proper contract
The abstract class correctly defines the userStats field, maintaining consistency with the implementation.
Also applies to: 281-281
Line range hint 1-290: Verify migration from previous stats structure
Since this is generated code reflecting a change from individual match stats to a consolidated userStats list, ensure that all consumers of this view model have been updated to use the new structure.
✅ Verification successful
Migration to new stats structure is properly handled
The code shows that both user_detail_screen.dart and user_stat_screen.dart have been updated to use the new userStats list structure, correctly extracting test and other stats from it. The component files (user_detail_batting_content.dart, user_detail_bowling_content.dart, and user_detail_fielding_content.dart) still use the old property names but they are now being passed the correctly transformed data from their parent components.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining references to the old stats structure
# Search for references to removed properties
echo "Checking for references to removed properties..."
rg -l "testMatchesCount|otherMatchesCount|testStats|otherStats" --type dart
# Search for potential usage of UserDetailViewState
echo "Checking for UserDetailViewState usage..."
rg -l "UserDetailViewState" --type dart
Length of output: 860
Script:
#!/bin/bash
# Let's examine the actual usage of these properties in the files
echo "Checking user_detail_fielding_content.dart..."
rg -A 5 -B 5 "testMatchesCount|otherMatchesCount|testStats|otherStats" khelo/lib/ui/flow/team/user_detail/component/user_detail_fielding_content.dart
echo -e "\nChecking user_detail_batting_content.dart..."
rg -A 5 -B 5 "testMatchesCount|otherMatchesCount|testStats|otherStats" khelo/lib/ui/flow/team/user_detail/component/user_detail_batting_content.dart
echo -e "\nChecking user_detail_bowling_content.dart..."
rg -A 5 -B 5 "testMatchesCount|otherMatchesCount|testStats|otherStats" khelo/lib/ui/flow/team/user_detail/component/user_detail_bowling_content.dart
echo -e "\nChecking user_detail_screen.dart..."
rg -A 5 -B 5 "testMatchesCount|otherMatchesCount|testStats|otherStats" khelo/lib/ui/flow/team/user_detail/user_detail_screen.dart
echo -e "\nChecking user_stat_screen.dart..."
rg -A 5 -B 5 "testMatchesCount|otherMatchesCount|testStats|otherStats" khelo/lib/ui/flow/stats/user_stat/user_stat_screen.dart
Length of output: 10823
khelo/lib/ui/flow/stats/user_stat/user_stat_view_model.dart (1)
3-4: Updated imports align with user statistics management
The addition of user_models.dart and user_service.dart imports correctly reflects the shift towards user-focused statistics, which is appropriate for the new functionality.
khelo/lib/ui/flow/team/user_detail/user_detail_view_model.dart (2)
49-49: LGTM!
The addition of _userService.streamUserStats(_userId!) effectively incorporates user statistics into the data stream.
55-55: LGTM!
Updating the state with userStats: event.$3 appropriately reflects the inclusion of user statistics in state management.
data/lib/service/user/user_service.dart (5)
56-62: LGTM!
The _userStatsRef method is correctly implemented, providing a collection reference for UserStat with appropriate Firestore converters.
114-115: LGTM!
The code correctly queries users by phone number.
171-181: LGTM!
The getUserStats method correctly retrieves user statistics filtered by type.
201-208: LGTM!
The updateUser method correctly updates the user data with merge options.
222-230: LGTM!
The _createUser method correctly creates a new user with the necessary fields.
data/lib/api/user/user_models.g.dart (8)
122-135: Deserialization of UserStatsImpl is correctly implemented
The _$$UserStatsImplFromJson function appropriately handles the deserialization of UserStatsImpl, with correct default values and null checks for nested objects.
137-144: Serialization of UserStatsImpl is accurately handled
The _$$UserStatsImplToJson function correctly serializes the UserStatsImpl instance to JSON, ensuring all fields are properly converted.
146-149: Enum mapping for UserStatType is correctly defined
The _$UserStatTypeEnumMap accurately maps UserStatType enum values to their string representations.
151-164: Deserialization of BattingImpl handles defaults appropriately
The _$$BattingImplFromJson function correctly deserializes BattingImpl objects, implementing default values and ensuring type safety with null-aware operators.
166-177: Serialization of BattingImpl is correctly implemented
The _$$BattingImplToJson function accurately serializes BattingImpl instances, ensuring all statistical fields are included in the JSON output.
180-192: Deserialization of BowlingImpl correctly manages default values
The _$$BowlingImplFromJson function properly deserializes BowlingImpl objects, with appropriate handling of nullable fields and default values.
194-206: Serialization of BowlingImpl is accurately handled
The _$$BowlingImplToJson function correctly serializes BowlingImpl instances, ensuring all relevant bowling statistics are included.
Line range hint 208-220: Deserialization and serialization of FieldingImpl are correctly implemented
The _$$FieldingImplFromJson and _$$FieldingImplToJson functions handle the conversion of FieldingImpl objects to and from JSON accurately, with proper default values.
data/lib/api/user/user_models.dart (1)
215-230: Potential division by zero in bowling statistics calculation
In the _addBowlingStats method, while calculating economyRate, there's a multiplication by 6 (assuming overs of 6 balls). However, if combinedBalls is zero, this could lead to a division by zero error when calculating the economy rate.
Please ensure that combinedBalls is not zero before performing the division.
data/lib/api/ball_score/ball_score_model.dart (1)
639-665: LGTM!
The calculateUserStats method correctly aggregates user statistics.
khelo/lib/ui/flow/score_board/score_board_view_model.dart (2)
Line range hint 307-317: Potential performance issue with sorting.
In the _loadBallScore method, you are sorting currentScoreList and previousScoreList using custom sort functions. Sorting can be resource-intensive on large lists. If the lists are large, consider using more efficient data structures or algorithms to handle sorted data.
[performance]
Consider whether the lists could be kept sorted as you insert elements, or whether a more efficient sorting algorithm is appropriate.
Line range hint 1117-1125: Possible null pointer exception in _handleAddingBall.
In the _handleAddingBall method, the switch statement might not handle all possible ScoreButton values, leading to potential issues if new enum values are added in the future.
Ensure all enum values are handled or include a default case to prevent runtime exceptions.
switch (btn) {
// existing cases...
default:
// Handle unexpected cases or throw an error
throw UnimplementedError('Button action not implemented for $btn');
}
khelo/lib/ui/flow/tournament/detail/tournament_detail_view_model.dart
Outdated
Show resolved
Hide resolved
khelo/lib/ui/flow/tournament/detail/tournament_detail_view_model.dart
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Outside diff range and nitpick comments (8)
data/lib/api/user/user_models.dart (2)
245-254: Add documentation for the updateStat methodThe
updateStatmethod performs complex statistical calculations. Consider adding documentation to explain:
- The purpose of the method
- How it combines two sets of statistics
- When it should be used
Add this documentation above the method:
+ /// Updates the current UserStat by combining it with another UserStat. + /// This is typically used when adding new match statistics to existing career statistics. + /// The method preserves the current stat type while combining all numerical values. UserStat updateStat(UserStat other) {
256-286: Consider extracting rate calculations into separate methodsThe calculation of averages, strike rates, and economy rates in both
_updateBattingand_updateBowlingmethods could be extracted into separate methods for better readability and maintainability.Consider refactoring like this:
+ double _calculateBattingAverage(int runs, int dismissals) => + dismissals == 0 ? 0.0 : runs / dismissals; + + double _calculateBattingStrikeRate(int runs, int balls) => + balls == 0 ? 0.0 : (runs / balls) * 100.0; + Batting _updateBatting(Batting oldBatting, Batting newBatting) { // ... existing combinations ... - final average = - combinedDismissals == 0 ? 0.0 : combinedRunsScored / combinedDismissals; - final strikeRate = combinedBallsFaced == 0 - ? 0.0 - : (combinedRunsScored / combinedBallsFaced) * 100.0; + final average = _calculateBattingAverage(combinedRunsScored, combinedDismissals); + final strikeRate = _calculateBattingStrikeRate(combinedRunsScored, combinedBallsFaced);Similar refactoring can be applied to bowling calculations.
Also applies to: 288-317
data/lib/api/ball_score/ball_score_model.dart (1)
778-780: Extract complex conditions into named boolean gettersThe complex conditions in where clauses could be more readable if extracted into named boolean getters.
Consider extracting conditions into named getters:
+ bool get isValidDeliveryForEconomyRate => + extras_type == null || + extras_type == ExtrasType.legBye || + extras_type == ExtrasType.bye; final bowledBallCountForEconomyRate = deliveries - .where( - (element) => (element.extras_type == null || - element.extras_type == ExtrasType.legBye || - element.extras_type == ExtrasType.bye), - ) + .where((element) => element.isValidDeliveryForEconomyRate)khelo/lib/ui/flow/score_board/score_board_view_model.dart (2)
Line range hint
1124-1136: Improve error handling specificityThe error handling in both
startNextInningandendMatchcould be more specific about what operation failed. This would help with debugging and user feedback.try { state = state.copyWith(actionError: null, isActionInProgress: true); final statsUpdated = await _addPlayerStats(); if (!statsUpdated) { - throw Exception("Failed to update player stats"); + throw Exception("Failed to update player stats: Stats update returned false"); } } catch (e) { debugPrint( - "ScoreBoardViewNotifier: error while start next inning -> $e"); + "ScoreBoardViewNotifier: ${e is Exception ? e.toString() : 'Unexpected error while updating player stats'} -> $e"); state = state.copyWith(actionError: e, isActionInProgress: false); }Also applies to: 1198-1209
783-791: Improve readability of stats calculation logicThe filtering logic for player stats could be made more maintainable by extracting the conditions into named predicates.
- .where( - (element) => - element.batsman_id == player || - element.bowler_id == player || - element.wicket_taker_id == player, - ) + .where((element) => _isPlayerInvolved(element, player)) + + bool _isPlayerInvolved(BallScoreModel ball, String playerId) { + return ball.batsman_id == playerId || + ball.bowler_id == playerId || + ball.wicket_taker_id == playerId; + }data/lib/api/user/user_models.freezed.dart (3)
1039-1046: Consider validating UserStatType with matches countThe
matchesfield has a default value of 0 whiletypeis nullable. Consider adding validation to ensure consistency between these fields, as having matches > 0 should likely correspond to a non-null type.@JsonSerializable(anyMap: true, explicitToJson: true) class _$UserStatImpl implements _UserStat { const _$UserStatImpl( {this.matches = 0, - this.type, + required this.type, this.batting = const Batting(), this.bowling = const Bowling(), this.fielding = const Fielding()});
1873-1876: Consider adding computed total dismissals and documentationThe fielding statistics could be enhanced with:
- A computed property for total dismissals
- Documentation for each field explaining what constitutes each type of dismissal
mixin _$Fielding { + /// Number of catches taken int get catches => throw _privateConstructorUsedError; + /// Number of run outs effected int get runOut => throw _privateConstructorUsedError; + /// Number of stumpings completed int get stumping => throw _privateConstructorUsedError; + + /// Total number of dismissals effected + int get total_dismissals => catches + runOut + stumping;Also applies to: 1980-1980
854-2053: Well-structured cricket statistics model hierarchyThe overall architecture of the statistics models is well thought out:
- Clear separation of batting, bowling, and fielding statistics
- Proper use of freezed for immutability
- Good serialization setup with explicit JSON handling
The models provide a solid foundation for tracking cricket statistics. The suggested improvements around computed properties and validation would make it even more robust.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (10)
data/lib/api/ball_score/ball_score_model.dart(5 hunks)data/lib/api/tournament/tournament_model.dart(2 hunks)data/lib/api/user/user_models.dart(1 hunks)data/lib/api/user/user_models.freezed.dart(1 hunks)data/lib/api/user/user_models.g.dart(1 hunks)khelo/lib/ui/flow/score_board/score_board_view_model.dart(6 hunks)khelo/lib/ui/flow/team/user_detail/component/user_detail_batting_content.dart(4 hunks)khelo/lib/ui/flow/team/user_detail/component/user_detail_bowling_content.dart(4 hunks)khelo/lib/ui/flow/tournament/detail/tabs/tournament_detail_stats_tab.dart(1 hunks)khelo/lib/ui/flow/tournament/detail/tournament_detail_view_model.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- data/lib/api/tournament/tournament_model.dart
- khelo/lib/ui/flow/team/user_detail/component/user_detail_bowling_content.dart
- khelo/lib/ui/flow/tournament/detail/tabs/tournament_detail_stats_tab.dart
- khelo/lib/ui/flow/tournament/detail/tournament_detail_view_model.dart
🔇 Additional comments (7)
khelo/lib/ui/flow/team/user_detail/component/user_detail_batting_content.dart (2)
Line range hint 1-100: Implementation looks good!
The widget implementation is clean and well-structured. The changes align well with the PR objectives of refactoring user stats classes and methods. The UI components properly handle nullable statistics and format the values appropriately.
1-1: Verify model refactoring across the codebase
The change from BattingStat to Batting type appears to be part of the larger refactoring effort. Let's ensure this change is consistent throughout the codebase.
Also applies to: 10-11
✅ Verification successful
Let me gather more context about the model changes to understand the full picture.
Let me modify the search patterns to get better results.
Model refactoring is correctly implemented
The verification shows that:
- The
Battingclass is properly defined indata/lib/api/user/user_models.dart - The old
BattingStatreferences inball_score_model.dartare part of the calculation logic that creates the newBattingmodel - The widget file correctly imports from
user_models.dartand uses the newBattingtype - Other files using batting statistics have also been updated to use the new model
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining references to the old BattingStat type
rg "BattingStat"
# Search for usage of the new Batting type to ensure consistent adoption
rg "Batting\?"
Length of output: 536
Script:
#!/bin/bash
# Check the implementation of Batting model
ast-grep --pattern 'class Batting'
# Check imports in the widget file to understand the model dependencies
rg -l "import.*ball_score_model\.dart" khelo/lib/ui/flow/team/user_detail/component/user_detail_batting_content.dart
# Check if there are any other files using the Batting model
rg -l "import.*user_models\.dart.*\n.*Batting" --type dart
Length of output: 408
Script:
#!/bin/bash
# Check the Batting model definition
rg "class Batting" -A 5
# Check all files importing user_models.dart
rg -l "import.*user_models\.dart"
# Check files importing ball_score_model.dart
rg -l "import.*ball_score_model\.dart"
Length of output: 5271
data/lib/api/ball_score/ball_score_model.dart (2)
639-665: LGTM! Well-structured user stats calculation.
The method effectively combines batting, bowling, and fielding statistics while properly handling unique match counting and optional parameter cases.
Line range hint 846-871: LGTM! Clean implementation of fielding statistics.
The method correctly handles different types of dismissals and properly aggregates fielding statistics.
data/lib/api/user/user_models.g.dart (3)
151-179: Ensure correct handling of calculations in BattingImpl
The BattingImpl class and its serialization methods are correctly implemented. Please ensure that calculations involving fields like average and strike_rate handle division by zero appropriately in the business logic to prevent runtime exceptions.
181-208: Verify calculations in BowlingImpl for potential divide-by-zero errors
In the BowlingImpl class, ensure that calculations for average, strike_rate, and economy_rate properly handle cases where division by zero could occur, avoiding runtime errors.
209-221: FieldingImpl serialization is correctly implemented
The FieldingImpl class and its serialization methods appear to be correctly implemented.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
data/lib/api/ball_score/ball_score_model.dart (3)
639-658: Add documentation and improve null-safetyConsider adding documentation to explain the purpose of each parameter and return value. Also, consider making the type parameter non-nullable if it's required for all stat calculations.
+ /// Calculates user statistics for cricket performance + /// + /// [currentUserId] The ID of the user whose stats are being calculated + /// [oldUserStats] Optional previous stats to update + /// [type] The type of statistics being tracked + /// [isMatchComplete] Whether the match is finished + /// Returns updated or new UserStat object UserStat calculateUserStats( String currentUserId, { UserStat? oldUserStats, - UserStatType? type, + required UserStatType type, bool isMatchComplete = false, })
762-765: Refactor duplicate delivery type conditionsConsider extracting the repeated conditions for legal deliveries and extras into helper methods or constants to improve maintainability and reduce duplication.
+ /// Returns true if the delivery is a legal delivery for bowling stats + bool isLegalDeliveryForBowling() { + return extras_type != ExtrasType.penaltyRun && + wicket_type != WicketType.retired && + wicket_type != WicketType.retiredHurt && + wicket_type != WicketType.timedOut; + } + /// Returns true if the delivery should be counted for economy rate + bool isValidForEconomyRate() { + return extras_type == null || + extras_type == ExtrasType.legBye || + extras_type == ExtrasType.bye; + }Also applies to: 771-773
Line range hint
839-864: Optimize fielding statistics calculationConsider combining the three separate where iterations into a single pass through the deliveries to improve performance, especially for large datasets.
Fielding calculateFieldingStats(String currentUserId) { + int catches = 0, runOuts = 0, stumpings = 0; + + for (final element in this) { + if (element.wicket_taker_id != currentUserId) continue; + + switch (element.wicket_type) { + case WicketType.caught: + case WicketType.caughtBehind: + case WicketType.caughtAndBowled: + catches++; + break; + case WicketType.runOut: + runOuts++; + break; + case WicketType.stumped: + stumpings++; + break; + default: + break; + } + } + + return Fielding( + catches: catches, + runOut: runOuts, + stumping: stumpings, + );khelo/lib/ui/flow/score_board/score_board_view_model.dart (1)
29-29: LGTM! Consider reordering service initialization.The addition of UserService as a dependency is well-structured. However, consider reordering the service initialization in the provider to group related services together.
return ScoreBoardViewNotifier( ref.read(matchServiceProvider), ref.read(inningServiceProvider), - ref.read(userServiceProvider), ref.read(ballScoreServiceProvider), + ref.read(userServiceProvider), );Also applies to: 37-37, 49-49
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
data/lib/api/ball_score/ball_score_model.dart(5 hunks)khelo/lib/ui/flow/score_board/score_board_view_model.dart(6 hunks)
🔇 Additional comments (5)
data/lib/api/ball_score/ball_score_model.dart (2)
726-726: Fix incorrect duck calculation logic
The current logic counts innings with 0-2 runs as ducks, which is incorrect. In cricket, a duck occurs only when a batsman is dismissed for 0 runs.
831-833: Fix maiden over calculation to check for legal deliveries
The current logic doesn't verify if all 6 balls in the over are legal deliveries. A maiden over must consist of 6 legal deliveries with no runs scored.
khelo/lib/ui/flow/score_board/score_board_view_model.dart (3)
1133-1133:
Await the player stats update
The call to _addPlayerStats() should be awaited to prevent race conditions and ensure stats are updated before proceeding with the inning change.
- _addPlayerStats();
+ final statsUpdated = await _addPlayerStats();
+ if (!statsUpdated) {
+ throw Exception("Failed to update player stats");
+ }Likely invalid or redundant comment.
1206-1206:
Await the player stats update
Similar to the previous issue, the call to _addPlayerStats() in endMatch should be awaited.
- _addPlayerStats(isMatchComplete: true);
+ final statsUpdated = await _addPlayerStats(isMatchComplete: true);
+ if (!statsUpdated) {
+ throw Exception("Failed to update player stats");
+ }Likely invalid or redundant comment.
775-801:
Critical issues in player stats update logic
The _addPlayerStats method has several issues that need to be addressed:
- Error handling is insufficient - errors are only logged but not propagated
- The method is called but not awaited in
startNextInningandendMatch - Missing null safety checks for
state.match?.players - No return value to indicate success/failure to callers
Consider applying these improvements:
- void _addPlayerStats({bool isMatchComplete = false}) async {
+ Future<bool> _addPlayerStats({bool isMatchComplete = false}) async {
try {
+ if (state.match?.players == null) {
+ return false;
+ }
final userStatType = state.match?.match_type == MatchType.testMatch
? UserStatType.test
: UserStatType.other;
for (final player in state.match?.players ?? []) {
final oldStats = await _userService.getUserStats(player, userStatType);
final newState = state.currentScoresList
.where(
(element) =>
element.batsman_id == player ||
element.bowler_id == player ||
element.wicket_taker_id == player,
)
.toList()
.calculateUserStats(player,
oldUserStats: oldStats, type: userStatType);
await _userService.updateUserStats(player, newState);
}
+ return true;
} catch (e) {
debugPrint(
"ScoreBoardViewNotifier: error while adding player stats -> $e");
+ state = state.copyWith(actionError: e);
+ return false;
}
}Likely invalid or redundant comment.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
data/lib/api/ball_score/ball_score_model.dart (4)
639-658: Add documentation and improve null-safety.Consider adding documentation to explain the purpose of each parameter and return value. Also, the null-safety could be improved.
+ /// Calculates user statistics for batting, bowling, and fielding. + /// + /// Parameters: + /// - currentUserId: The ID of the user whose stats are being calculated + /// - oldUserStats: Optional previous stats to be updated + /// - type: Optional type of the statistics + /// - isMatchComplete: Whether the match is complete (defaults to false) + /// + /// Returns a new or updated UserStat object. UserStat calculateUserStats( String currentUserId, { UserStat? oldUserStats, UserStatType? type, bool isMatchComplete = false, }) { - final newBattingStat = calculateBattingStats(currentUserId); + final Batting newBattingStat = calculateBattingStats(currentUserId); - final newBowlingStat = calculateBowlingStats(currentUserId); + final Bowling newBowlingStat = calculateBowlingStats(currentUserId); - final newFieldingStat = calculateFieldingStats(currentUserId); + final Fielding newFieldingStat = calculateFieldingStats(currentUserId);
772-779: Simplify filtering conditions by extracting common logic.The filtering conditions are repeated in multiple places and could be simplified by extracting them into a separate method.
+ bool _isValidDeliveryForStats(BallScoreModel ball) { + return (ball.extras_type == null || + ball.extras_type == ExtrasType.legBye || + ball.extras_type == ExtrasType.bye) && + ball.wicket_type != WicketType.retired && + ball.wicket_type != WicketType.retiredHurt && + ball.wicket_type != WicketType.timedOut && + ball.extras_type != ExtrasType.penaltyRun; + } final bowledBallCountForEconomyRate = deliveries - .where( - (element) => - (element.extras_type == null || - element.extras_type == ExtrasType.legBye || - element.extras_type == ExtrasType.bye) && - element.wicket_type != WicketType.retired && - element.wicket_type != WicketType.retiredHurt && - element.wicket_type != WicketType.timedOut && - element.extras_type != ExtrasType.penaltyRun, - ) + .where(_isValidDeliveryForStats) .length;
819-841: Add documentation explaining maiden over rules.The method implements the maiden over calculation correctly, but adding documentation would help future maintainers understand the cricket-specific rules.
+ /// Calculates the number of maiden overs bowled. + /// + /// A maiden over in cricket is when no runs are scored off the bat + /// and no extras are conceded in a complete over of 6 legal deliveries. int _calculateMaidenOvers(Iterable<BallScoreModel> deliveries) {
Line range hint
844-869: Add type annotations for better readability.The method implementation is correct, but adding explicit type annotations would improve code readability.
Fielding calculateFieldingStats(String currentUserId) { - final catches = where( + final int catches = where( (element) => element.wicket_taker_id == currentUserId && (element.wicket_type == WicketType.caught || element.wicket_type == WicketType.caughtBehind || element.wicket_type == WicketType.caughtAndBowled), ).length; - final runOuts = where( + final int runOuts = where( (element) => element.wicket_taker_id == currentUserId && element.wicket_type == WicketType.runOut, ).length; - final stumpings = where( + final int stumpings = where( (element) => element.wicket_taker_id == currentUserId && element.wicket_type == WicketType.stumped, ).length;
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
data/lib/api/ball_score/ball_score_model.dart (3)
639-657: Add documentation for the calculateUserStats method.Consider adding documentation to explain the purpose of each parameter and the method's behavior, especially the update mechanism when oldUserStats is provided.
+ /// Calculates user statistics for cricket performance. + /// + /// Parameters: + /// - currentUserId: The ID of the user whose stats are being calculated + /// - oldUserStats: Optional existing stats to update + /// - type: Optional type of the statistics (e.g., T20, ODI) + /// - isMatchComplete: Whether the match is complete, affects matches count + /// + /// Returns a new or updated UserStat instance. UserStat calculateUserStats( String currentUserId, { UserStat? oldUserStats,
817-839: Optimize maiden over calculation.The current implementation can be made more efficient by:
- Early exit when ball count isn't 6
- Using any() instead of a flag variable for extras
int _calculateMaidenOvers(Iterable<BallScoreModel> deliveries) { final overGroups = groupBy(deliveries, (ball) => ball.over_number); int maiden = 0; overGroups.forEach((overNumber, balls) { + if (balls.length != 6) return; int runsConceded = 0; - bool hasExtras = false; + final hasExtras = balls.any((ball) => ball.extras_type != null); + if (hasExtras) return; for (final ball in balls) { runsConceded += ball.runs_scored + (ball.extras_awarded ?? 0); - if (ball.extras_type != null) { - hasExtras = true; - break; - } } - if (runsConceded == 0 && !hasExtras && balls.length == 6) { + if (runsConceded == 0) { maiden++; } }); return maiden; }
Line range hint
842-867: Optimize fielding statistics calculation.Consider combining similar where clauses to reduce code duplication and improve maintainability.
Fielding calculateFieldingStats(String currentUserId) { - final catches = where( + final dismissals = where( (element) => - element.wicket_taker_id == currentUserId && - (element.wicket_type == WicketType.caught || - element.wicket_type == WicketType.caughtBehind || - element.wicket_type == WicketType.caughtAndBowled), - ).length; - - final runOuts = where( - (element) => - element.wicket_taker_id == currentUserId && - element.wicket_type == WicketType.runOut, - ).length; - - final stumpings = where( - (element) => - element.wicket_taker_id == currentUserId && - element.wicket_type == WicketType.stumped, - ).length; + element.wicket_taker_id == currentUserId + ).toList(); + + final catches = dismissals.where((element) => + element.wicket_type == WicketType.caught || + element.wicket_type == WicketType.caughtBehind || + element.wicket_type == WicketType.caughtAndBowled).length; + + final runOuts = dismissals.where((element) => + element.wicket_type == WicketType.runOut).length; + + final stumpings = dismissals.where((element) => + element.wicket_type == WicketType.stumped).length; return Fielding( catches: catches, runOut: runOuts, stumping: stumpings, ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
data/lib/api/ball_score/ball_score_model.dart(6 hunks)
🔇 Additional comments (2)
data/lib/api/ball_score/ball_score_model.dart (2)
725-725:
Fix duck calculation logic.
A duck in cricket occurs only when a batsman is dismissed without scoring any runs (0 runs). The current implementation incorrectly considers scores between 0 and 2 as ducks.
- if (isDismissed && inningRunScored >= 0 && inningRunScored <= 2) {
+ if (isDismissed && inningRunScored == 0) {761-764:
Revise ball count logic for economy rate.
The current implementation includes penalty runs in the ball count, which is incorrect as these don't count as actual deliveries.
- element.extras_type != ExtrasType.penaltyRun &&
element.wicket_type != WicketType.retired &&
element.wicket_type != WicketType.retiredHurt &&
element.wicket_type != WicketType.timedOut,Likely invalid or redundant comment.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
data/lib/api/ball_score/ball_score_model.dart (4)
639-657: Consider enhancing null-safety.The method handles null cases well but could be more explicit about nullability:
- UserStat calculateUserStats( + UserStat calculateUserStats( String currentUserId, { UserStat? oldUserStats, UserStatType? type, bool isMatchComplete = false, -}) { +}) { + if (currentUserId.isEmpty) { + throw ArgumentError('currentUserId cannot be empty'); + } final newBattingStat = calculateBattingStats(currentUserId); // ... rest of the method
Line range hint
751-822: Extract common filtering conditions.The method has duplicate conditions across multiple where clauses. Consider extracting these into named predicates for better maintainability and readability.
+ bool _isValidDelivery(BallScoreModel ball) => + ball.wicket_type != WicketType.retired && + ball.wicket_type != WicketType.retiredHurt && + ball.wicket_type != WicketType.timedOut && + ball.extras_type != ExtrasType.penaltyRun; Bowling calculateBowlingStats(String currentUserId) { final deliveries = where((element) => element.bowler_id == currentUserId); final wicketTaken = deliveries .where( - (element) => - element.wicket_type != null && - element.wicket_type != WicketType.retired && - element.wicket_type != WicketType.retiredHurt && - element.wicket_type != WicketType.timedOut, + (element) => element.wicket_type != null && _isValidDelivery(element), ) .length; // ... rest of the method
Line range hint
849-874: Improve readability with named predicates.The method uses multiple where clauses with similar conditions. Consider extracting these into named predicates for better readability.
+ bool _isCatch(BallScoreModel ball) => + ball.wicket_type == WicketType.caught || + ball.wicket_type == WicketType.caughtBehind || + ball.wicket_type == WicketType.caughtAndBowled; Fielding calculateFieldingStats(String currentUserId) { final catches = where( - (element) => - element.wicket_taker_id == currentUserId && - (element.wicket_type == WicketType.caught || - element.wicket_type == WicketType.caughtBehind || - element.wicket_type == WicketType.caughtAndBowled), + (element) => element.wicket_taker_id == currentUserId && _isCatch(element), ).length; // ... rest of the method
Duck calculation logic needs correction
The current implementation considers a duck when a player is dismissed after facing 0-2 balls, which is incorrect according to cricket rules. A duck occurs when a batsman is dismissed without scoring any runs. The number of balls faced is not relevant for determining a duck.
Let's verify if there are any runs scored in the innings:
- if (isDismissed && inningBallFaced >= 0 && inningBallFaced <= 2) { + if (isDismissed && inningRunScored == 0) { ducks++; }🔗 Analysis chain
Line range hint
660-748: Verify duck calculation logic and consider method extraction.
The duck calculation logic at line 736 might need verification. In cricket, a duck typically means getting out for zero runs, but the current implementation checks for 0-2 balls faced.
The method is quite long and handles multiple responsibilities. Consider extracting the innings analysis into a separate method.
Consider extracting the innings analysis:
+ Batting _analyzeInnings(Map<String, List<BallScoreModel>> inningGroup, String currentUserId) { + int ducks = 0, fifties = 0, hundreds = 0; + // ... move innings analysis logic here + return Batting(ducks: ducks, fifties: fifties, hundreds: hundreds); + }🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other duck implementations in the codebase rg -A 5 "duck|Duck" --type dartLength of output: 10665
user-stats.webm
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Refactor