-
Notifications
You must be signed in to change notification settings - Fork 0
FTP #9
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?
Conversation
dzharkov
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.
Пока 5/10
| os.flush(); | ||
|
|
||
| final int number = is.readInt(); | ||
| System.out.println("Total number of files: " + number); |
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.
А можно сделать интерфейс клиента таким, чтобы его можно было безболезненно переиспользовать для GUI?
Пока -1
| throw new FileAlreadyExistsException(); | ||
| } | ||
| final byte[] buf = new byte[BUF_SIZE]; | ||
| try (final FileOutputStream os = new FileOutputStream(file)) { |
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.
Давайте попытаемся избежать ручного манипулирования байтовыми буферами и воспользуемся IOUtils из внешней библиотеки?
Пока -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.
Исправил. Для отправки файла тоже можно было бы использовать такой способ, но, к сожалению, описанный интерфейс абстрактной сессии не позваляет. Менять же его ради одноразовай задачи не считаю логичным.
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.
Мне скорее реализация нетривиальной логики руками кажется не очень логичной.
Пока по-прежнему -1
| // all docs inherited from Session interface | ||
| @Override | ||
| public void run() { | ||
| new Thread(this::processRead).start(); |
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.
По-хорошему нужно избешать создания потоков для каждой сессии. Иначе рост их количества быстро приводит к деградации JVM. Пока -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.
Да, лечится это общим тред-пулом
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.
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.
Аппеляция к конспекту засчитана, хоть мне все еще не кажется это правильным.
Баллы вернул
| this.socket = socket; | ||
| this.id = id; | ||
| this.server = server; | ||
| executor = Executors.newSingleThreadExecutor(); |
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.
В чем смысл отдельного потока для подкоманд?
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.
См. комментарий выше.
|
Извиняюсь, 4/9 |
|
Частично исправил. Некоторые Ваши замечания меня смущают...
2018-05-30 21:01 GMT+03:00 Denis Zharkov <notifications@github.com>:
… Извиняюсь, 4/9
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#9 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AP3e77zuRxRTe2ecEFg9MnlhLi8TSH48ks5t3t6BgaJpZM4UQC-y>
.
|
dzharkov
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.
Пока текущая оценка 8
Макс. оценка 9
|
А можно занести это в табличку?
2018-06-02 15:44 GMT+03:00 Denis Zharkov <notifications@github.com>:
… ***@***.**** requested changes on this pull request.
Пока текущая оценка 8
Макс. оценка 9
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#9 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AP3e79uINSkzfs632ZwdDn6l4rIYGqunks5t4oidgaJpZM4UQC-y>
.
|
|
У меня нет к ней доступа. Обратитесь, пожалуйста, к Тимофею или Юрию |

No description provided.