Hashtable #1
Hashtable #1
Conversation
yurii-litvinov
left a comment
yurii-litvinov
left a comment
There was a problem hiding this comment.
По тестам и комментариям существенных замечаний нет, саму хеш-таблицу можно улучшить (немного, правда, она и так почти ок)
| } | ||
|
|
||
| /** | ||
| * @return number of keys in hashtable |
There was a problem hiding this comment.
Лучше без тэга @return --- так этот комментарий сгенерится только в детальное описание метода, а если без тэга, то и в краткое описание. Как сейчас, краткое описание будет пустым
| */ | ||
| public String put(String key, String value) { | ||
| int hash = Math.floorMod(key.hashCode(), capacity); | ||
| String return_value = hashtable[hash].put(key, value); |
There was a problem hiding this comment.
В Java принят стиль camelCase для именования переменных, так что по джавовскому стайлгайду правильно returnType
| */ | ||
| public void clear() { | ||
| for (int i = 0; i < capacity; i++) | ||
| hashtable[i] = new LinkedList(); |
There was a problem hiding this comment.
Лучше заключать в фигурные скобки тело, даже если там один оператор. Во избежание ошибок вида
for (int i = 0; i < capacity; i++)
System.out.println(i);
hashtable[i] = new LinkedList();| * Singly linked list for hashtable. | ||
| */ | ||
| public class LinkedList { | ||
| Node head = new Node(null, null); |
There was a problem hiding this comment.
Если не указывать модификатор видимости, видимость будет пакетная. Что, наверное, для головы списка не очень :)
| /** | ||
| * Adds new node to list, increments size. | ||
| */ | ||
| private void add(Node new_node) { |
There was a problem hiding this comment.
Тут тоже, лучше newNode
| */ | ||
| class Node { | ||
| Node next; | ||
| String key, value; |
There was a problem hiding this comment.
Тут тоже с видимостью полей всё плохо
| @@ -0,0 +1,155 @@ | |||
| package ru.spbau.mit.kazakov.HashTable.test; | |||
There was a problem hiding this comment.
Тесты лучше в отдельную папку на одном уровне с src, иначе системы сборки их не найдут
yurii-litvinov
left a comment
yurii-litvinov
left a comment
There was a problem hiding this comment.
Существенных замечаний больше нет, зачтена полностью
| * Node for linked list. | ||
| * Store hashtable's key and value | ||
| */ | ||
| class Node { |
There was a problem hiding this comment.
он должен бы быть private static
| */ | ||
| class Node { | ||
| private Node next; | ||
| private String key, value; |
There was a problem hiding this comment.
Лучше каждое поле объявлять отдельно. Вот, например, key по смыслу final (менять ключ в записи в хеш-таблице --- очень плохая идея), а value --- нет. В такой форме записи это не записать.
| /** | ||
| * Node constructor | ||
| * | ||
| * @param key key in hash table |
There was a problem hiding this comment.
Лучше не выравнивать по вертикали, это первого же рефакторинга не переживёт
| class HashTableTest { | ||
| @Test | ||
| void testConstructor() { | ||
| HashTable hashtable = new HashTable(); |
There was a problem hiding this comment.
Переменная в этом тесте не нужна, достаточно просто вызвать конструктор и посмотреть, что он не падает
| @@ -0,0 +1,153 @@ | |||
| import org.junit.jupiter.api.Test; | |||
There was a problem hiding this comment.
Обычно тесты в том же пакете, что и тестируемый класс, чтобы иметь доступ к его пакетным методам
No description provided.