-
Notifications
You must be signed in to change notification settings - Fork 6k
Fix Collection builder example to be well-behaved with proper buffer allocation #47994
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: BillWagner <493969+BillWagner@users.noreply.github.com>
Co-authored-by: BillWagner <493969+BillWagner@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good start, but the code needs more updates.
docs/csharp/language-reference/operators/snippets/shared/CollectionExpressionExamples.cs
Outdated
Show resolved
Hide resolved
docs/csharp/language-reference/operators/snippets/shared/CollectionExpressionExamples.cs
Outdated
Show resolved
Hide resolved
…ctionExpressionExamples.cs
@copilot Can you address the remaining comment? |
Co-authored-by: BillWagner <493969+BillWagner@users.noreply.github.com>
Fixed the buffer allocation issue. The LineBuffer now allocates exactly the buffer size needed instead of a fixed 80-character array. Changes made in commit 425a8b2:
The collection now properly allocates only the memory it needs while maintaining the well-behaved collection behavior. |
docs/csharp/language-reference/operators/snippets/shared/operators.csproj
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is now ready for final review
The
LineBuffer
class in the Collection builder example was not well-behaved according to the documentation's own definition. It had two main issues:Enumeration mismatch: When creating a collection with a few elements, it would enumerate all 80 elements from its internal buffer, violating the principle that "The value of
Count
orLength
on a countable collection produces the same value as the number of elements when enumerated."Memory waste: It always allocated a fixed 80-character buffer regardless of actual usage, wasting memory for smaller collections.
Before this fix:
After this fix:
Changes made:
_count
field to track the actual number of elements set in the constructornew char[80]
to dynamicnew char[buffer.Length]
Count
property to make the collection countableGetEnumerator()
to only yield actual elements usingyield return
this[int index]
with proper bounds checking for direct element accessIEnumerable.GetEnumerator()
to delegate to the generic versionThe collection now properly demonstrates the "well-behaved collection" concept with efficient memory usage and correct enumeration behavior.
Fixes #41756.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.