-
Notifications
You must be signed in to change notification settings - Fork 0
test #5
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
base: master
Are you sure you want to change the base?
test #5
Conversation
yurii-litvinov
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.
Тестов нет, а по условию нужны были
| Microsoft Visual Studio Solution File, Format Version 12.00 | ||
| Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Test1.1", "Test1.1\Test1.1.csproj", "{E451040C-44C5-427F-9CF7-F83F90E54DFF}" | ||
| EndProject | ||
| Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Test1.1.Tests", "Test1.1.Tests\Test1.1.Tests.csproj", "{E628CC17-DD0A-4658-8561-CB4C496A798D}" |
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.
Даже CI не одобряет отсутствия тестов
| /// </summary> | ||
| public class Client | ||
| { | ||
| private TcpClient _client; |
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.
Если хоть одно поле у класса IDisposable, то и сам класс должен быть IDisposable, и у себя в Dispose вызывать Dispose всех своих IDisposable-полей. Иначе есть риск, что Dispose у них не вызовется до самого конца программы (что сродни утечке памяти).
|
|
||
| public Client(string host, int port) | ||
| { | ||
| _client = new TcpClient(host, port); |
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.
Это сразу инициирует подключение, и если хост не ответит, приведёт к ожиданию таймаута, который в случае с TCP может быть довольно большим. И всё это время вызывающий будет заблокирован. Надо было использовать ConnectAsync, и не в конструкторе, а при старте.
| await Task.Run(async () => | ||
| { | ||
| var writer = new StreamWriter(stream) { AutoFlush = true }; | ||
| while (true) |
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.
Итого у Вас три (!) вложенных бесконечных цикла, тогда как работает (и нужен) только один
| Task.Run(async () => | ||
| { | ||
| while (true) { | ||
| await Writer(_client.GetStream()); |
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.
Не-а, так Reader будет заблокирован. Надо было Task.WhenAny
| } | ||
| else | ||
| { | ||
| var client = new Client(args[0], Int32.Parse(args[1])); |
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.
Никто не обещал, что будет хоть один аргумент, кстати
| public class Server | ||
| { | ||
| private int _port; | ||
| private readonly CancellationTokenSource _cancellationToken = new CancellationTokenSource(); |
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.
| private readonly CancellationTokenSource _cancellationToken = new CancellationTokenSource(); | |
| private readonly CancellationTokenSource _cancellationToken = new (); |
Вы ж не на Java пишете :)
| } | ||
| } | ||
| }); | ||
| } |
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.
Что-то копипаст, такое в клиенте уже было
Test1.1/Test1.1/Program.cs
Outdated
| else | ||
| { | ||
| var client = new Client(args[0], Int32.Parse(args[1])); | ||
| client.Working(); |
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.
await server.Working(); но блокирующий client.Working();, несимметрично и неправильно (клиент тоже может быть асинхронным, конечно)
| Console.WriteLine(data + "\n"); | ||
| if (data == "exit") | ||
| { | ||
| StopServer(); |
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.
Выйти из чата может только сервер, кажется :)
No description provided.