-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Missing using for StreamReader variable #8558
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
Conversation
The example includes: using WebClient client = new WebClient(); using Stream data = client.OpenRead(args[0]); But was missing: StreamReader reader = new StreamReader(data);
|
Tagging subscribers to this area: @dotnet/ncl Issue DetailsSummaryThe example includes: But was missing: Fixes #Issue_Number (if available)
|
|
Learn Build status updates of commit 1453f9b: ✅ Validation status: passed
For more details, please refer to the build report. Note: Broken links written as relative paths are included in the above build report. For broken links written as absolute paths or external URLs, see the broken link report. For any questions, please:
|
|
My goal when changing the documentation is to change as little as possible.
The Close methods were there to start but there was inconsistent use of the
using keyword. Also, the Close methods were ordered incorrectly.
When I personally code, I explicitly include the Close method such as with
Streams. I do not explicitly call Dispose for a class like WebClient which
does not implement a Close.
…On Sun, Oct 23, 2022 at 12:42 PM Anton Firszov ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In snippets/csharp/System.Net/WebClient/Overview/useragent.cs
<#8558 (comment)>
:
> data.Close();
reader.Close();
Do we still need the explicit Close() calls?
—
Reply to this email directly, view it on GitHub
<#8558 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AFAZFDFMIID4FT3KU2KFTCTWEWIJHANCNFSM6AAAAAARL2KUKA>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
I think it would be still better to delete the explicit |
Per @anton's comments, removed Close methods as the using performs the close.
|
Learn Build status updates of commit 664d8a1: ✅ Validation status: passed
For more details, please refer to the build report. Note: Broken links written as relative paths are included in the above build report. For broken links written as absolute paths or external URLs, see the broken link report. For any questions, please:
|
BillWagner
left a comment
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.
Thanks for making this change @softwarepronto
I like where this landed. I'll
now.
Summary
The example includes:
using WebClient client = new WebClient();
using Stream data = client.OpenRead(args[0]);
But was missing:
StreamReader reader = new StreamReader(data);
Fixes #Issue_Number (if available)