Skip to content

Conversation

@pvktk
Copy link
Owner

@pvktk pvktk commented Dec 17, 2018

No description provided.

pvktk added 22 commits December 7, 2018 19:53
Сделаны join потоков сервера.
Добавлена команда resume.
До этого для него выделялся новый id.
Copy link

@sproshev sproshev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

10

/**
*
*/
private static final long serialVersionUID = 1L;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

зачем? оно ж не сериализуется

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Это подсказка IDE была.


ExecutorService pool = Executors.newCachedThreadPool();

Map<Integer, Future<?>> fileDownloadsFutures = new HashMap<>();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

модификаторы доступа? не, не слышал)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

если сделали старт, но не делали стоп, получается мапа будет содержать выполненные future сколь угодно долго?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Исправил.


public void stopFileDownload(int fileId) {
if (!filesHolder.fileStatus.containsKey(fileId)) {
throw new NullPointerException("This file wasn't been downloading");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

это исключение про разыменование нулевого указателя, здесь скорее IllegalArgumentException/IllegalStateException/NoSuchElementException

try {
return Math.toIntExact((files.get(fileId).length() + pieceSize - 1) / pieceSize);
} catch (IOException e) {
throw new RuntimeException(e);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

можно еще java.io.UncheckedIOException

}
}

public int pieceOffset(int fileId, int numPart) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

не используется

}
} catch (IOException e) {
out.println(e.getMessage());
} catch (InterruptedException e) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

возможно лучше finally

} catch (IOException e) {
// TODO Auto-generated catch block
e.printStackTrace();
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

не очень хорошая обработка IOException
семафор может быть не релизнут и/или канал не закрыт

dout.writeByte(1);

fileSizes.clear();
filesNames.clear();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

лучше все-таки не менять передаваемые параметры, а результат работы "возвращать"

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Но тут параметров много.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

и?) это же не оправдывает, в плюсах так исторически сложилось, что можно, в джаве скорее нет

dinp.close();
dout.close();
s.close();
} catch (NullPointerException npe) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

это совсем беда, NPE это ошибка программирования, и оно никак не может оказаться в ожидаемых исключениях, вместо этого должна использоваться проверка на null

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Кажется что это красивее чем делать три if и пытаться сообщить что кто-то оказался null.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

сокет закрывает свои потоки

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

да и try-with-resources есть

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Сделал вместо этого проверку на null.

dout = new DataOutputStream(s.getOutputStream());
dinp = new DataInputStream(s.getInputStream());

dout.writeByte(1);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

flush

В частности сделал загрузку файлового дискриптора ленивой
(только когда надо считать или записать кусок).

Future из мапы в FilesDownloader удаляется при завершении загрузки.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants