doc(IDynamicObject): update table dynamic data sample#7289
Merged
Conversation
Contributor
Reviewer's GuideRefactors the dynamic table IDynamicObject sample to use initialized backing fields instead of nullable properties, introduces helper methods to generate richer typed dynamic data for both static and time-based columns, simplifies Razor bindings accordingly, and generalizes CustomDynamicData to support object-valued columns with more efficient lookups. Sequence diagram for generating static dynamic-object table rowssequenceDiagram
actor User
participant Browser
participant TablesDynamicObject
participant CustomDynamicColumnsObjectData
User->>Browser: Navigate_to_/table/dynamic-object
Browser->>TablesDynamicObject: Initialize_component
activate TablesDynamicObject
TablesDynamicObject->>TablesDynamicObject: OnInitialized()
TablesDynamicObject->>TablesDynamicObject: _dynamicColumnList = time_based_strings
TablesDynamicObject->>TablesDynamicObject: _customDynamicItems = GenerateDynamicColumnsObjectData()
loop index 1..10
TablesDynamicObject->>TablesDynamicObject: GenerateRowData(index)
TablesDynamicObject-->>TablesDynamicObject: rowData Dictionary~string, object?~
TablesDynamicObject->>CustomDynamicColumnsObjectData: new(fix:index, columns:rowData)
CustomDynamicColumnsObjectData-->>TablesDynamicObject: CustomDynamicColumnsObjectData_instance
end
TablesDynamicObject-->>Browser: Render_two_tables_with_dynamic_columns
deactivate TablesDynamicObject
Sequence diagram for querying time-based dynamic columnssequenceDiagram
actor User
participant Browser
participant TableComponent as Table_CustomDynamicData
participant TablesDynamicObject
participant CustomDynamicData
User->>Browser: Open_dynamic_time_based_table
Browser->>TableComponent: Request_data
TableComponent->>TablesDynamicObject: OnQueryAsync(options)
activate TablesDynamicObject
loop index 1..10
TablesDynamicObject->>TablesDynamicObject: GenerateDynamicRowData(index)
TablesDynamicObject-->>TablesDynamicObject: rowData Dictionary~string, object?~
TablesDynamicObject->>CustomDynamicData: new(fix:index, data:rowData)
CustomDynamicData-->>TablesDynamicObject: CustomDynamicData_instance
end
TablesDynamicObject-->>TableComponent: QueryData~CustomDynamicData~(Items, TotalCount, IsSorted, IsFiltered)
deactivate TablesDynamicObject
TableComponent-->>Browser: Render_rows_with_random_values_per_time_column
Updated class diagram for dynamic table sample and CustomDynamicDataclassDiagram
class TablesDynamicObject {
-IEnumerable~CustomDynamicColumnsObjectData~ _customDynamicItems
-List~string~ _dynamicColumnList
+OnInitialized() void
-GenerateDynamicColumnsObjectData() IEnumerable~CustomDynamicColumnsObjectData~
-GenerateRowData(index int) Dictionary~string, object?~
-GenerateDynamicRowData(index int) Dictionary~string, object?~
-StaticColumnList List~string~$
-random Random$
-OnQueryAsync(options QueryPageOptions) Task~QueryData~CustomDynamicData~~
}
class CustomDynamicData {
+Fix string
+Columns Dictionary~string, object?~
+CustomDynamicData(fix string, data Dictionary~string, object?~)
+TryGetMember(binder GetMemberBinder, result out object?) bool
}
class CustomDynamicColumnsObjectData {
+Fix string
+Columns Dictionary~string, object?~
+CustomDynamicColumnsObjectData(fix string, columns Dictionary~string, object?~)
}
class QueryData_CustomDynamicData {
+Items IEnumerable~CustomDynamicData~
+TotalCount int
+IsSorted bool
+IsFiltered bool
}
class QueryPageOptions {
+PageIndex int
+PageItems int
}
class DynamicObject {
}
TablesDynamicObject "1" --> "*" CustomDynamicColumnsObjectData
TablesDynamicObject "1" --> "*" CustomDynamicData
TablesDynamicObject --> QueryData_CustomDynamicData
TablesDynamicObject --> QueryPageOptions
CustomDynamicData --|> DynamicObject
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- In
GenerateRowData,StaticColumnListis a property that creates a new list on each access, so repeatedly callingStaticColumnList.CountandStaticColumnList[i]will allocate multiple lists and can break indexing; cache it in a local variable (e.g.,var columns = StaticColumnList;) and loop over that instead. GenerateDynamicRowDatatakes anindexparameter but never uses it, so either remove the parameter or incorporate it into the generated values to avoid confusion.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `GenerateRowData`, `StaticColumnList` is a property that creates a new list on each access, so repeatedly calling `StaticColumnList.Count` and `StaticColumnList[i]` will allocate multiple lists and can break indexing; cache it in a local variable (e.g., `var columns = StaticColumnList;`) and loop over that instead.
- `GenerateDynamicRowData` takes an `index` parameter but never uses it, so either remove the parameter or incorporate it into the generated values to avoid confusion.
## Individual Comments
### Comment 1
<location> `src/BootstrapBlazor.Server/Components/Samples/Table/TablesDynamicObject.razor.cs:69` </location>
<code_context>
+ return ret;
}
private readonly static Random random = new();
private Task<QueryData<CustomDynamicData>> OnQueryAsync(QueryPageOptions options)
</code_context>
<issue_to_address>
**issue (bug_risk):** Consider using a thread-safe random strategy instead of a shared static Random instance.
Because `Random` isn’t thread-safe, a shared static instance can behave incorrectly under concurrent calls to `OnQueryAsync` (e.g., contention, repeated sequences). Prefer `RandomNumberGenerator` for cryptographic needs, or `Random.Shared` / `ThreadLocal<Random>` for non-cryptographic randomness, depending on your requirements.
</issue_to_address>
### Comment 2
<location> `src/BootstrapBlazor.Server/Components/Samples/Table/TablesDynamicObject.razor.cs:79-66` </location>
<code_context>
return Task.FromResult(new QueryData<CustomDynamicData>() { Items = items, TotalCount = 10, IsSorted = true, IsFiltered = true });
}
+
+ private Dictionary<string, object?> GenerateDynamicRowData(int index)
+ {
+ var ret = new Dictionary<string, object?>();
+ for (int i = 0; i < _dynamicColumnList.Count; i++)
+ {
+ var columnName = _dynamicColumnList[i];
+ object? value = random.Next(1000, 9999);
+ ret.Add(columnName, value);
+ }
+ return ret;
+ }
}
</code_context>
<issue_to_address>
**suggestion:** The index parameter of GenerateDynamicRowData is currently unused.
Since `index` isn’t used, the signature and call `GenerateDynamicRowData(index)` are misleading. Either use `index` in the generated data if it’s meant to influence the row, or remove the parameter and adjust the call site so it’s clear the row data doesn’t depend on the row index.
Suggested implementation:
```csharp
var items = Enumerable.Range(1, 10).Select(index => new CustomDynamicData(index.ToString(), GenerateDynamicRowData()));
```
```csharp
private Dictionary<string, object?> GenerateDynamicRowData()
```
None required beyond this file, as the only visible call site has been updated and the removed parameter was unused in the method body.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| } | ||
| ret.Add(columnName, value); | ||
| } | ||
| return ret; |
Contributor
There was a problem hiding this comment.
suggestion: The index parameter of GenerateDynamicRowData is currently unused.
Since index isn’t used, the signature and call GenerateDynamicRowData(index) are misleading. Either use index in the generated data if it’s meant to influence the row, or remove the parameter and adjust the call site so it’s clear the row data doesn’t depend on the row index.
Suggested implementation:
var items = Enumerable.Range(1, 10).Select(index => new CustomDynamicData(index.ToString(), GenerateDynamicRowData())); private Dictionary<string, object?> GenerateDynamicRowData()None required beyond this file, as the only visible call site has been updated and the removed parameter was unused in the method body.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7289 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 745 745
Lines 32628 32628
Branches 4522 4522
=========================================
Hits 32628 32628
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the table dynamic data sample to demonstrate more flexible data types by changing from Dictionary<string, string> to Dictionary<string, object?>. This allows the sample to showcase various data types (string, int, DateTime, bool) in dynamic columns.
- Changed
CustomDynamicData.Columnstype fromDictionary<string, string>toDictionary<string, object?>to support multiple data types - Refactored nullable properties with
[NotNull]attributes to initialized private fields following modern C# practices - Optimized
TryGetMemberto useTryGetValuepattern instead ofContainsKeyfollowed by indexer access - Removed redundant template markup from razor components, simplifying the column definitions
- Added new helper methods to generate sample data with diverse types (strings, integers, dates, booleans)
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/BootstrapBlazor.Server/Data/CustomDynamicData.cs | Changed dictionary value type from string to object? and optimized member access with TryGetValue |
| src/BootstrapBlazor.Server/Components/Samples/Table/TablesDynamicObject.razor.cs | Refactored to use private fields, added helper methods for generating diverse sample data types |
| src/BootstrapBlazor.Server/Components/Samples/Table/TablesDynamicObject.razor | Removed redundant template markup and updated field references to match code-behind changes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| else if (columnName == "Z") | ||
| { | ||
| value = i % 2 == 0; |
There was a problem hiding this comment.
The boolean value is calculated using the loop counter i instead of the row index. This means all rows will have the same boolean pattern based on the StaticColumnList position, not varying per row. Consider changing to value = index % 2 == 0; to make the boolean value dependent on the row number.
Suggested change
| value = i % 2 == 0; | |
| value = index % 2 == 0; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Link issues
fixes #7288
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Revise the dynamic table IDynamicObject sample to generate richer, strongly-typed dynamic column data and simplify column binding in the demo.
New Features:
Bug Fixes:
Enhancements: