Soft cache by nrsxx · Pull Request #2 · nrsxx/java-tasks

nrsxx · 2017-04-23T15:08:21Z

No description provided.

DanAnastasyev · 2017-04-23T20:47:02Z

SoftCache/src/SoftCacheMap.java

+import java.util.HashMap;
+import java.util.HashSet;
+
+public class SoftCacheMap implements Cache<Integer, String> {


А почему не сделать универсальную реализацию?

DanAnastasyev · 2017-04-23T20:47:52Z

SoftCache/src/SoftCacheMap.java

+public class SoftCacheMap implements Cache<Integer, String> {
+
+    private HashMap<Integer, SoftReference<String>> hashMap1;
+    private HashMap<SoftReference<String>, HashSet<Integer>> hashMap2;


Не очень-то говорящие названия

DanAnastasyev · 2017-04-23T20:52:09Z

SoftCache/src/SoftCacheMap.java

+        } else {
+            hashMap2.put(hashMap1.get(key), new HashSet(key));
+        }
+        while (delete()) {


Почему не сделать метод cleanup (например), который будет выполнять всю чистку. delete - достаточно размытое название, а пустой цикл смотрится совсем странно. Ну, и вызывать метод много раз вместо того, чтоб пробежаться в одном цикле по всем элементам referenceQueue - не слишком-то практично

DanAnastasyev · 2017-04-23T21:03:23Z

SoftCache/src/SoftCacheMap.java

+        if (hashMap2.get(hashMap1.get(key)) != null) {
+            hashMap2.get(hashMap1.get(key)).add(key);
+        } else {
+            hashMap2.put(hashMap1.get(key), new HashSet(key));


Зачем столько звать hashMap1.get(key)? Ты же сам только что туда положил этот элемент.

DanAnastasyev · 2017-04-23T21:05:55Z

SoftCache/.idea/misc.xml

@@ -0,0 +1,6 @@
+<?xml version="1.0" encoding="UTF-8"?>


Все эти *.xml и .iml файлы тоже не очень-то нужны в репозитории. Они специфичны конкретно для твоей машины, остальным про них знать ничего не надо

DanAnastasyev · 2017-04-23T21:08:21Z

SoftCache/src/SoftCacheMap.java

+import java.util.HashMap;
+import java.util.HashSet;
+
+public class SoftCacheMap implements Cache<Integer, String> {


Вообще говоря, в задании ещё была речь про небольшой тестик, демонстрирующий работу. А ещё про то, что хорошо бы препятствовать вытеснению объектов, к которым недавно обращались. То есть можно было бы держать небольшую очередь с Strong Reference на объекты, которые только что запрашивали.

DanAnastasyev · 2017-04-23T21:09:40Z

SoftCache/src/SoftCacheMap.java

+        SoftReference reference = (SoftReference) referenceQueue.poll();
+        if (reference != null) {
+            if (hashMap1.entrySet().contains(reference)) {
+                HashSet localSet = hashMap2.remove(reference);


Дженериковые классы создавай с их параметрами: HashSet<Integer>. Тогда бы цикл был не по непонятным Object
К конструктору SoftCacheMap это тоже относится. Idea же даже посвечивать должна такое

DanAnastasyev · 2017-04-23T21:11:57Z

SoftCache/src/SoftCacheMap.java

+        if (hashMap1.get(key).get() != null) {
+            return hashMap1.get(key).get();
+        }
+        return null;


Тогда уж можно было бы просто return hashMap1.get(key).get(); написать. Хотя на наличие ключа в мапе тоже стоит проверять

DanAnastasyev · 2017-04-23T21:12:41Z

SoftCache/src/SoftCacheMap.java

+    @Override
+    public void put(Integer key, String value) {
+        hashMap1.put(key, new SoftReference(value, referenceQueue));
+        if (hashMap2.get(hashMap1.get(key)) != null) {


Если что, есть метод containsKey

DanAnastasyev · 2017-04-23T21:14:54Z

SoftCache/src/SoftCacheMap.java

+
+    @Override
+    public String remove(Integer key) {
+        if (hashMap1.get(key).get() != null) {


Всегда проверяй на наличие ключа, прежде чем использовать полученное значение. NullPointerException в Java самая распространенная ошибка

DanAnastasyev

Напиши хоть какой-нибудь код, демонстрирующий, что это работает

DanAnastasyev · 2017-04-24T13:17:50Z

SoftCache/src/SoftCacheMap.java

 import java.util.HashSet;

-public class SoftCacheMap implements Cache<Integer, String> {
+public class SoftCacheMap implements Cache<Integer, Object> {


Не. У тебя же шаблонный интерфейс. Почему бы не сделать шаблонную же реализацию?

DanAnastasyev · 2017-04-24T13:19:12Z

SoftCache/src/SoftCacheMap.java

+        while (!done) {
+            SoftReference reference = (SoftReference) referenceQueue.poll();
+            if (reference != null) {
+                if (cache.entrySet().contains(reference)) {


А что возвращает entrySet(), кстати?

Set из пар <key, value>. Тут, видимо, нужен values() - он возвращает коллекцию из значений

А еще лучше сделать if (subsidiaryMap.containsKey(reference))

DanAnastasyev · 2017-04-24T18:55:09Z

SoftCache/src/SoftCacheMap.java

+        if (subsidiaryMap.containsKey(reference)) {
+            subsidiaryMap.get(reference).add(key);
+        } else {
+            subsidiaryMap.put(reference, new HashSet<>(key));


new HashSet<>(key) - что, по-твоему, тут происходит?

DanAnastasyev · 2017-04-25T15:17:58Z

SoftCache/src/SoftCacheMap.java

+import java.util.HashSet;
+import java.util.LinkedList;
+
+public class SoftCacheMap<V> implements Cache<Integer, V> {


Что же всё-таки мешает тебе сделать его SoftCacheMap<K, V>? Зачем тебе там Integer?

DanAnastasyev · 2017-04-25T15:23:06Z

SoftCache/src/SoftCacheMap.java

+
+    private HashMap<Integer, SoftReference<V>> cache;
+    private HashMap<SoftReference<V>, HashSet<Integer>> subsidiaryMap;
+    private ReferenceQueue<? super Object> referenceQueue;


А что означает <? super Object>, и зачем тебе это тут?

Ограничение, означает что мы при обращении к элементам очереди получим Object, а записать в очередь можем любого наследника Object.

А с учетом того, что все классы - наследники Object, не проще ли написать ReferenceQueue<Object>?
Но с учетом того, что в ней могут быть только объекты типа V, напиши лучше ReferenceQueue<V>
И почитай на всякий случай вот тут про wildcards в шаблонах

DanAnastasyev · 2017-04-27T22:16:51Z

SoftCache/src/SoftCacheMap.java

+        cache1.put(0, new Integer(0));
+
+        /**
+         * должен быть 0


Вообще говоря, в задании говорилось про написания юнит-теста. Тут же так и просится assert вместо фразы "должен быть 0".
Перенеси это всё в класс с junit-тестом. Можешь почитать где-нибудь тут или посмотреть в основном репозитории, как они делаются

nrsxx added 3 commits April 19, 2017 14:03

SoftCache task

4e815f5

Binary files deleted

b5a6e3b

ReferenceQueue checking added

2a08199

DanAnastasyev suggested changes Apr 23, 2017

View reviewed changes

nrsxx added 2 commits April 24, 2017 14:28

Fixes

694ea72

Delete unnecessary files

01f750b

DanAnastasyev reviewed Apr 24, 2017

View reviewed changes

Queue of recently used objects added

47cd948

DanAnastasyev reviewed Apr 24, 2017

View reviewed changes

nrsxx added 3 commits April 25, 2017 00:35

Fixes

b63aea2

Small fixes + test added

0f58e75

Small fixes

031dec1

DanAnastasyev reviewed Apr 25, 2017

View reviewed changes

nrsxx added 2 commits April 25, 2017 20:45

Fixes

0da56dd

Fixes

475e34d

DanAnastasyev reviewed Apr 27, 2017

View reviewed changes

Unit-test added

6ebecef

Conversation

nrsxx commented Apr 23, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DanAnastasyev left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants