TcpClient.GetStream() sample code is incorrect and confusing

[geoffkizer]

See https://docs.microsoft.com/en-us/dotnet/api/system.net.sockets.tcpclient.getstream?view=net-6.0

There are several issues here:

(1) The sample incorrectly states twice that "Closing the tcpClient instance does not close the network stream." This is wrong: closing the TcpClient does in fact close the NetworkStream. The TcpClient.Close docs are clear about this.
(2) The sample calls GetStream() without having established the connection first. This will always fail.
(3) The sample explicitly tests for CanRead and CanWrite, but these will always be true on an established connection, so checking for these is very confusing.
(4) The sample explicitly closes the TcpClient in failure cases but not in the success case. This is bizarre and makes it unclear whether the user needs to call TcpClient.Close in general or not.
(5) The sample uses tcpClient.ReceiveBufferSize as the size of the user's allocated buffer for Receive. This seems to imply that this is good practice or even required; it is neither. TcpClient.ReceiveBufferSize configures the kernel buffer size for the socket and doesn't really have anything to do with the choice of user buffer size on Receive.

Additionally, the sample should be updated to use Span and using/IDisposable as any modern code would do.

If someone can tell me how to update the sample myself, I'd be happy to do so.

[ghost]

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

---

See https://docs.microsoft.com/en-us/dotnet/api/system.net.sockets.tcpclient.getstream?view=net-6.0

There are several issues here:

(1) The sample incorrectly states twice that "Closing the tcpClient instance does not close the network stream." This is wrong: closing the TcpClient does in fact close the NetworkStream. The TcpClient.Close docs are clear about this.
(2) The sample calls GetStream() without having established the connection first. This will always fail.
(3) The sample explicitly tests for CanRead and CanWrite, but these will always be true on an established connection, so checking for these is very confusing.
(4) The sample explicitly closes the TcpClient in failure cases but not in the success case. This is bizarre and makes it unclear whether the user needs to call TcpClient.Close in general or not.
(5) The sample uses tcpClient.ReceiveBufferSize as the size of the user's allocated buffer for Receive. This seems to imply that this is good practice or even required; it is neither. TcpClient.ReceiveBufferSize configures the kernel buffer size for the socket and doesn't really have anything to do with the choice of user buffer size on Receive.

Additionally, the sample should be updated to use Span and using/IDisposable as any modern code would do.

If someone can tell me how to update the sample myself, I'd be happy to do so.

Author:	geoffkizer
Assignees:	-
Labels:	documentation, area-System.Net.Sockets
Milestone:	-

[filipnavara]

Did you mean this one: https://github.com/dotnet/dotnet-api-docs/blob/1dcbb0b6ec5614b59d15bd4aecb557ad07668fff/samples/snippets/csharp/VS_Snippets_Remoting/ClassicTcpClient.PublicMethodsAndPropertiesExample/CS/source.cs#L154-L198 ?

All the documentation is in that repository. You can click the "Edit" button in the upper right corner of the MSDN page and find the relevant method. The snippets are file links with "SnippetXY" tag. The files are in the same repository and each snippet is framed by a comment. You can submit PRs there.

[geoffkizer]

Yes, that's what I was looking for, thanks @filipnavara!