Conversation
yurii-litvinov
left a comment
yurii-litvinov
left a comment
There was a problem hiding this comment.
О, я Вам, оказывается, ревью не отправил, а Вы и не спросили про замечания :)
| }, | ||
| }; | ||
|
|
||
| this.server.Start(); |
There was a problem hiding this comment.
Сурово. Можно было создать объект-сервер в этом же процессе, они бы нормально общались с клиентом по сети. Хотя так более показательно, так что можно не править
| public void Cleanup() | ||
| { | ||
| var dir = TestContext.CurrentContext.TestDirectory; | ||
| foreach (var file in new[] { "file.txt", "pin.jpg", "123.docx", "111.txt" }) |
There was a problem hiding this comment.
| foreach (var file in new[] { "file.txt", "pin.jpg", "123.docx", "111.txt" }) | |
| foreach (var file in ["file.txt", "pin.jpg", "123.docx", "111.txt"]) |
| RunClient("2 ./Test/file.txt"); | ||
| var path = Path.Combine(TestContext.CurrentContext.TestDirectory, "file.txt"); | ||
| Assert.That(File.Exists(path), Is.True, "file.txt did not download!"); | ||
| } |
There was a problem hiding this comment.
Стоило проверить и его содержимое, вдруг там пустой файл просто создаётся
| Assert.Multiple(() => | ||
| { | ||
| Assert.That(File.Exists(downloaded), Is.True, "pin.jpg did not download!"); | ||
| Assert.That(GetSha256(downloaded), Is.EqualTo(originalHash)); |
| const string host = "127.0.0.1"; | ||
| const int port = 12345; |
There was a problem hiding this comment.
Я бы это принимал аргументом командной строки
| while (true) | ||
| { | ||
| Console.Write("\n> "); | ||
| var input = Console.ReadLine()?.Trim(); |
There was a problem hiding this comment.
И я бы ожидал от таких инструментов меньше интерактива — всё передаём из командной строки, получаем вывод в stdout, с пользователем не общаемся. Так в скриптах удобнее (например, wget так работает, поэтому вполне может вызываться, например, посреди сборки в CI).
| } | ||
|
|
||
| throw new IOException("Unexpected end of stream while reading file size"); | ||
| } No newline at end of file |
There was a problem hiding this comment.
Это какой-то Python-стайл — всё в одном файле, ничего не переиспользуемо, UI и бизнес-логика в одну кучу. На C# так не принято писать, хочется тут отдельный класс, который можно было бы использовать в CLI и тестировать, и если мы захотим вдруг написать UI или веб-обёртку над всем этим, мы бы могли легко и без модификаций кода это сделать.
| { | ||
| var client = await listener.AcceptTcpClientAsync(); | ||
| _ = Task.Run(() => HandleClientAsync(client)); | ||
| } |
There was a problem hiding this comment.
Тут надо предусмотреть механизм корректного выключения сервера. Ну и в целом, все асинхронные операции по-хорошему должны быть прерываемыми, и если они прерываемые, дать пользователю их прервать из CLI.
| { | ||
| var rel = Path.GetRelativePath(root, fullPath).Replace('\\', '/'); | ||
| return rel == "." ? "." : rel.StartsWith("./") ? rel : "./" + rel; | ||
| } No newline at end of file |
There was a problem hiding this comment.
Тут тоже, Python-стайл какой-то. От Вас в курсе по C# ожидаются переиспользуемые компоненты, оформленные по возможности в соответствии с гайдлайнами по написанию библиотек, принятых в C#. Тут хочется архитектурный рефакторинг.
2 participants
No description provided.