Conversation
yurii-litvinov
left a comment
yurii-litvinov
left a comment
There was a problem hiding this comment.
В CI тест не прошёл. Но в целом ок, плюс-минус проблемы с корректной остановкой и некоторые небольшие проблемы со стилем кодирования. 7 баллов из 10.
| <Project Sdk="Microsoft.NET.Sdk"> | ||
|
|
||
| <PropertyGroup> | ||
| <TargetFramework>net7.0</TargetFramework> |
There was a problem hiding this comment.
.NET 7 умер. .NET 9 Preview 2 даже давно доступен. Переходите на .NET 8
| using System.Net; | ||
| using System.Net.Sockets; | ||
|
|
||
| public class Client |
| /// <summary> | ||
| /// Starts client, that writes messages from concole, communicates witn server and stops when get "exit" from server or from comsole. | ||
| /// </summary> | ||
| /// <returns></returns> |
| var stream = client.GetStream(); | ||
| var writer = new StreamWriter(stream) { AutoFlush = true }; | ||
| var reader = new StreamReader(stream); |
There was a problem hiding this comment.
Кто-то из них должен быть объявлен с using, чтобы корректно закрыть поток
| async void actionWriteEntered() => await WriteEntered(writer, lockObject); | ||
| async void actionReadPrintResponse() => await ReadPrintResponse(reader, lockObject); |
There was a problem hiding this comment.
Нет нужды именованные функции объявлять. Так можно, но можно просто лямбды.
| lock (lockObject) | ||
| { | ||
| Console.WriteLine(message); | ||
| } |
There was a problem hiding this comment.
Зачем, мы ведь его только что ввели. Вообще, Console.WriteLine внутри синхронизируется, lockObject не нужен
|
|
||
|
|
||
|
|
||
|
|
| /// Starts client, that writes messages from concole, communicates witn server and stops when get "exit" from server or from comsole. | ||
| /// </summary> | ||
| /// <returns></returns> | ||
| public async Task Start() |
There was a problem hiding this comment.
По соглашению, принятому в .NET, все async-методы должны иметь суффикс "Async"
| { | ||
| object lockObject = new(); | ||
| listener.Start(); | ||
| var tcpClient = await listener.AcceptTcpClientAsync(); |
There was a problem hiding this comment.
Вызывать длительную операцию без способа её отменить может быть плохой идеей, потому что можно задедлочиться. Тут, например, если к нам так и не подключится никакой клиент, AcceptTcpClientAsync никогда не вернёт управление и остановить сервер будет нельзя. Чтобы так не было, у AcceptTcpClientAsync есть перегрузка, принимающая CancellationToken.
| { | ||
| if (message == "exit") | ||
| { | ||
| IsInterrupted = true; |
There was a problem hiding this comment.
Тут тоже, если IsInterrupted true, то ReadPrintResponse не закончит свою работу, пока клиент нам что-нибудь не пришлёт. Потенциальный дедлок.
2 participants
No description provided.