-
Notifications
You must be signed in to change notification settings - Fork 79
proskurina-596g-task 3 #320
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
| @@ -1,50 +1,165 @@ | |||
| package ru.mipt.java2016.homework.g596.proskurina.task2; | |||
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.
Лучше перенеси новое в .task3, в .task2 оставь всё, как оно было во втором задании
| private InputStream readBuffer = null; | ||
| private OutputStream writeBuffer = null; | ||
|
|
||
| private long currentPositionInStream = 0; |
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.
В java по умолчанию поля инициализируются именно так
| this.keySerialiser = keySerialiser; | ||
| this.valueSerialiser = valueSerialiser; | ||
| private Long currentPositionInValuesFile = new Long(0); | ||
| private final Integer lock = 42; |
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.
Вот такой Integer очень опасно использовать как лок
| try { | ||
| String inputData = file.read(fileName); | ||
| private LoadingCache<K, V> cacheValues = CacheBuilder.newBuilder() | ||
| .maximumSize(42) |
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.
Выноси такие константы как static final поля
| public int size() { | ||
| checkIfFileIsOpen(); | ||
| return map.size(); | ||
| synchronized (lock) { |
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.
У тебя получается однопоточная реализация этого метода: одновременно только один поток может узнавать размер. С ReadWriteLock можно быстрее
| int bytesNumber = ByteBuffer.wrap(bytesNumberArray).getInt(); | ||
| byte[] bytesArray = new byte[bytesNumber]; | ||
| int bytesArraySize = readBuffer.read(bytesArray, 0, bytesNumber); | ||
| //addToCalc(bytes); |
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.
Не бросай закомментированный код в репозитории. Ему так грустно и одиноко от этого
| valuesFile.close(); | ||
| valuesFile.delete(); | ||
| newValuesFile.rename(directoryPath + "valuesFile.db"); | ||
| newValuesFile.close(); |
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.
FileWorker же Closeable. Используй try-with-resources и тогда close() вызовется автоматически и при любых неожиданностях
| writeToFileDeleteKeySet(); | ||
| deleteKeyFile.close(); | ||
| keyPositionFile.close(); | ||
| valuesFile.close(); |
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.
Повторяются строчки
deleteKeyFile.close();
keyPositionFile.close();тут и в rebuild. Почему бы их просто не вынести из if?
|
|
||
| @Override | ||
| public boolean exists(K key) { | ||
| wlock.lock(); |
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.
А почему тут wlock?
И в readKeys
No description provided.