- HomeWork4 #58
- HomeWork4 #58
Conversation
NikitaBabich26rus
commented
Nov 8, 2020
NikitaBabich26rus
commented
- Added Tests
- Added comments
- Added Tests - Added comments
| public async Task<byte[]> Get(string path) | ||
| { | ||
| var client = new TcpClient(host, port); | ||
| using (var stream = client.GetStream()) |
There was a problem hiding this comment.
Можно using var, который без скобок
| var client = new TcpClient(host, port); | ||
| using (var stream = client.GetStream()) | ||
| { | ||
| var writer = new StreamWriter(stream) { AutoFlush = true }; |
There was a problem hiding this comment.
Он тоже IDisposable, поэтому тоже надо using var
| var writer = new StreamWriter(stream) { AutoFlush = true }; | ||
| await writer.WriteLineAsync("1"); | ||
| await writer.WriteLineAsync(path); | ||
| var reader = new StreamReader(stream); |
| { | ||
| var writer = new StreamWriter(stream) { AutoFlush = true }; | ||
| await writer.WriteLineAsync("1"); | ||
| await writer.WriteLineAsync(path); |
There was a problem hiding this comment.
Это нарушение требуемого протокола, в условии запрос пишется в одну строчку. Ваш клиент не будет работать с чужими серверами.
|
|
||
| if (size == -1) | ||
| { | ||
| Console.WriteLine("This file does not exist."); |
There was a problem hiding this comment.
Нельзя прямо в клиенте выводить на консоль ничего, потому что вдруг отсутствие файла вполне ожидаемо и не надо пугать пользователя. Вывод на консоль вообще по идее должен быть только в Program
| public async Task Start() | ||
| { | ||
| listener.Start(); | ||
| while (!cancellationTokenSource.IsCancellationRequested) |
There was a problem hiding this comment.
Здесь надо CancellationToken использовать по идее
| { | ||
| var reader = new StreamReader(stream); | ||
| var writer = new StreamWriter(stream) { AutoFlush = true }; | ||
| var comand = await reader.ReadLineAsync(); |
| break; | ||
|
|
||
| default: | ||
| Console.WriteLine("Input error."); |
There was a problem hiding this comment.
И тоже не стоит ничего на консоль писать прямо тут, можно бросать исключение или сделать себе мини-логгер, который сюда принимать и который бы уже на консоль выводил (а лучше log4et подрубить)
| } | ||
|
|
||
| var content = new byte[size]; | ||
| await reader.BaseStream.ReadAsync(content, 0, size); |
There was a problem hiding this comment.
Не стоит зачитывать весь файл в память, файлы могут быть существенно больше, чем Вам бы хотелось. Принимайте поток и выводите в него, пусть он сам разберётся.
| [Test] | ||
| public async Task GetWithIncorrectFileNameTest() | ||
| { | ||
| server.Start(); |
There was a problem hiding this comment.
server.Start(); делается везде, можно было его в Before вынести
| } | ||
| currentIndex++; | ||
| } | ||
| await stream.CopyToAsync(fileStream); |
There was a problem hiding this comment.
Не слишком надёжный способ, потому что не учитывает переданного вначале размера файла, и если соединение разорвётся посередине передачи, оно спокойно сохранит половину файла и даже не поругается. Но ладно, можно и так.
Правильнее было бы вычитывать по кусочкам в буфер, проверять, что всё ок, и скидывать на диск.
| while (!cancellationTokenSource.Token.IsCancellationRequested) | ||
| { | ||
| var client = await listener.AcceptTcpClientAsync(); | ||
| await Task.Run(() => Working(client)); |
There was a problem hiding this comment.
Если здесь делается await, мы не сможем получить следующий client до того, как закончим работу с этим. Что портит многопоточность
