Skip to content

Conversation

@wfurt
Copy link
Member

@wfurt wfurt commented Apr 12, 2022

To point made in #63162 - TcpClient is really useless in most cases. In this case it really does not provide anything beyond GetStream() and it costs extra object to be allocated.

@wfurt wfurt requested a review from a team April 12, 2022 00:50
@wfurt wfurt self-assigned this Apr 12, 2022
@ghost
Copy link

ghost commented Apr 12, 2022

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

Issue Details

To point made in #63162 - TcpClient is really useless in most cases. In this case it really does not provide anything beyond GetStream() and it costs extra object to be allocated.

Author: wfurt
Assignees: wfurt
Labels:

area-System.Net

Milestone: -

Copy link
Member

@gfoidl gfoidl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some nits.

int port = _uri.Port;

TcpClient client = new TcpClient();
var client = new Socket(SocketType.Stream, ProtocolType.Tcp); ;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
var client = new Socket(SocketType.Stream, ProtocolType.Tcp); ;
var client = new Socket(SocketType.Stream, ProtocolType.Tcp);

Copy link
Member

@ManickaP ManickaP left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apart from the nits pointed out by @gfoidl looks good.

@wfurt wfurt merged commit 5bf13e9 into dotnet:main Apr 13, 2022
@wfurt wfurt deleted the tcpClient branch April 13, 2022 01:53
@ghost ghost locked as resolved and limited conversation to collaborators May 13, 2022
@karelz karelz added this to the 7.0.0 milestone Jul 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants