Skip to content

Test3 - #8

Open
ArtemNikit1n wants to merge 2 commits into
mainfrom
test3
Open

Test3#8
ArtemNikit1n wants to merge 2 commits into
mainfrom
test3

Conversation

@ArtemNikit1n

Copy link
Copy Markdown
Owner

No description provided.

clientStream.Position = 0;
var receivedByClient = await clientReader.ReadLineAsync();
Assert.That(receivedByClient, Is.EqualTo(serverMessage));
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Вы потестировали библиотечные StreamReader и StreamWriter, тут собственно "боевой" код нигде не упоминается даже

if (serverIp == null)
{
throw new ArgumentNullException(nameof(serverIp));
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Используйте ArgumentNullException.ThrowIfNull. На три строчки короче.

try
{
Console.WriteLine($"Подключение к серверу {serverIp}:{port}...");
var client = new TcpClient();

Copy link
Copy Markdown

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 TcpClient();
using var client = new TcpClient();

Comment on lines +37 to +40
await using (var stream = client.GetStream())
using (var reader = new StreamReader(stream, Encoding.UTF8))
await using (var writer = new StreamWriter(stream, Encoding.UTF8))
{

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Тут можно было без скобок, тогда бы объекты дожили до конца блока (думаю, что Console.WriteLine пережил бы, если бы они были живы)


var sendTask = SendMessagesAsync(writer);

await Task.WhenAny(receiveTask, sendTask);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

По-хорошему надо корректно остановить и дождаться завершения обеих задач

using System.Net;
using System.Net.Sockets;
using System.Text;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

А тут надо было предупреждения StyleCop поправить

{
try
{
var listener = new TcpListener(IPAddress.Any, port);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

using тоже

Console.WriteLine($"Сервер запущен на порту {port}");
Console.WriteLine("Ожидание подключения клиента...");

var client = await listener.AcceptTcpClientAsync();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Тоже надо было сделать отменяемым, а то вдруг к нам никто так и не подключится, оно будет висеть тут вечно (справедливости рад, чтобы это было можно отменить, нужна некоторая дополнительная машинерия в отдельном потоке, которую не надо было реализовывать на контрольной — но сейчас оно неотменяемо by design, и даже если захотеть, эту машинерию не сделать).

{
Console.WriteLine($"\nОшибка при отправке клиенту: {ex.Message}");
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

На стороне клиента то же самое, надо было унифицировать как-то. По смыслу циклы передачи-приёмки сообщений на сервере и на клиенте никак не отличаются.

if (message.ToLower() == "exit")
{
Console.WriteLine("\nСервер завершил соединение");
break;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Нас уведомляют, что сервер завершил соединение, но из цикла ввода не выходят, так что попытка что-то отправить закончится исключением Broken pipe. А всё потому, что Вы CancellationToken не используете.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants