Simple ftp #6
Simple ftp #6
Conversation
dzharkov
left a comment
dzharkov
left a comment
There was a problem hiding this comment.
Текущая оценка: 4.5
Максимальная: 10
| byte[] buffer = new byte[4096]; | ||
| long remaining = length; | ||
| int read; | ||
| while ((read = in.read(buffer, 0, (int) Math.min(buffer.length, remaining))) > 0) { |
There was a problem hiding this comment.
Вместо этого лучше воспользоваться IOUtils из common-io
There was a problem hiding this comment.
попробовал IOUtils.copy, получилось удобно, но, к сожалению, так можно передать только один файл, а потом стрим нужно закрыть, соответственно, придется закрывать соединение. Поискал способы передавать файлы, не закрывая стрим, рекомендуют мой старый способ. Поэтому оставляю старый
There was a problem hiding this comment.
Не очень понял, какой стрим должен закрыться
https://commons.apache.org/proper/commons-io/javadocs/api-2.5/org/apache/commons/io/IOUtils.html#copy(java.io.InputStream,%20java.io.OutputStream)
Здесь вроде про это не написано и в коде реализации я не вижу ничего про закрытие
There was a problem hiding this comment.
Как IOUtils.copy на клиенте поймёт, что приём файла закончился?
Возможно, можно отправлять размер файла, но copy просто считывает входной стрим до конца, а копирования стольки-то байт я не нашёл.
There was a problem hiding this comment.
| out.println(fromUser); | ||
| } | ||
| fromServer = ResponseCode.values()[in.readInt()]; | ||
| switch (fromServer) { |
There was a problem hiding this comment.
Предлагаю воспользоваться технологиями ООП и вынести логику клиента в отдельный класс, отделив ее от работы с консольным вводом-выводом.
Это автоматически поможет с тестами.
Пока -1
| } | ||
| case FOLDER_DESCRIPTION: { | ||
| FolderDescription description = FolderDescription.read(in); | ||
| description.print(); |
There was a problem hiding this comment.
Я бы предпочел, чтобы FolderDescription просто возвращал описание файлов в удобном для принтинга виде.
Тогда в тот момент, когда мы добавим GUI, его не придется переписывать.
Пока -0.5
| /** | ||
| * Client handler, runs in a separate thread as long as there is a connectin with a client. | ||
| */ | ||
| private static class FTPThread extends Thread { |
There was a problem hiding this comment.
Вместо наследования потока правильней передать в конструктор реализацию Runnable.
Пока -1
| out.writeLong(file.length()); | ||
| byte[] buffer = new byte[4096]; | ||
| int read; | ||
| try (FileInputStream in = new FileInputStream(file)) { |
| while (true) { | ||
| try { | ||
| Socket clientSocket = serverSocket.accept(); | ||
| new FTPThread(clientSocket).start(); |
There was a problem hiding this comment.
Вообще, ручное создание потоков инстансов Thread -- сомнительная практика. В частности это может привести к тому, что будет создано неконтролируемое число потоков, что приведет к деградации производительности JVM.
Проще всего воспользоваться тредпулом из java.util.concurrent.
Пока -1
|
|
||
| import org.junit.Test; | ||
|
|
||
| public class FolderDescriptionTest { |
There was a problem hiding this comment.
Тестов по сути нет
Максимальная оценка снижается на 1, минимальная еще на 1
|
Пока оценка 8/9 (из-за IOUtils, который можно исправить и тестов, за которые снижена макс. оценка) |
No description provided.