-
Notifications
You must be signed in to change notification settings - Fork 0
Hash table test #3
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
| /** | ||
| * Creates a HashTable with given capacity. | ||
| */ | ||
| public HashTable (int capacity) { |
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.
Перед "(" тут пробел не ставится
| for (int i = 0; i < capacity; i++) { | ||
| storage[i] = new List(); | ||
| } | ||
| size = 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 сама проинициализирует поле значением по умолчанию, так что это писать не обязательно
| * @param key The given key | ||
| * @return The index in the array. | ||
| **/ | ||
| public int getIndex (String key) { |
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.
Хмм, зачем бы это public?
| int index = getIndex(key); | ||
| return storage[index].get(key); | ||
| } | ||
| /** |
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.
Надо пустую строчку между методом и комментарием к следующему
| public String remove (String key) { | ||
| int index = getIndex(key); | ||
| if (storage[index].contains(key)) | ||
| size--; |
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.
Лучше заключать тело в фигурные скобки
| public void contains() throws Exception { | ||
| HashTable table = new HashTable(4); | ||
| table.put("1", "v1"); | ||
| assert(table.contains("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.
А тут, соответственно, assertTrue
| table.put("2", "v2"); | ||
| assert(table.get("1").equals("v1")); | ||
| assert(table.get("2").equals("v2")); | ||
| assert(table.get("3") == null); |
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.
А тут assertNull :)
src/ru/spbau/dkaznacheev/List.java
Outdated
| /** | ||
| * Subclass that describes the key-value pair. | ||
| */ | ||
| public class Pair { |
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-way. И его можно сделать static
| * The first element of the list. | ||
| **/ | ||
| private Node head; | ||
| /** |
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.
Тут тоже, надо пустую строку между полем и комментарием к следующему
| size = 1; | ||
| } | ||
|
|
||
| public int size() { |
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.
И ко всем паблик-методам (и конструкторам) нужны комментарии
yurii-litvinov
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.
Есть некоторые замечания, которые я бы хотел, чтобы Вы поправили, но там немного, поэтому давайте считать, что задача про тесты зачтена, а первую надо поправить
| /** | ||
| * Current capacity of the table. | ||
| **/ | ||
| private int capacity; |
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.
Подозреваю, что capacity всегда равна storage.length, так что её в принципе можно не хранить
| **/ | ||
| public void clear() { | ||
| for (int i = 0; i < capacity; i++) | ||
| storage[i] = new List(); |
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.
Лучше у всех операторов типа if, for и т.д. заключать тело в фигурные скобки, даже если там один оператор, во избежание ошибок в духе
for (int i = 0; i < capacity; i++)
System.out.println("Creating list");
storage[i] = new List();| assertEquals(0, table.size()); | ||
| } | ||
|
|
||
|
|
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.
Лишняя пустая строчка :)
| HashTable table = new HashTable(4); | ||
| table.put("1", "v1"); | ||
| assertTrue(table.contains("1")); | ||
| assertTrue(!table.contains("2")); |
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.
Тут лучше assertFalse
| @@ -0,0 +1,222 @@ | |||
| package ru.spbau.dkaznacheev; | |||
|
|
|||
| import com.sun.org.apache.bcel.internal.generic.LUSHR; | |||
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.
Мм?
| /** | ||
| * Key of pair | ||
| */ | ||
| private String key; |
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.
Мне кажется, key лучше сделать final, всё равно менять его прямо по ходу работы --- плохая идея
| * @param key key of node | ||
| * @param value value of node | ||
| */ | ||
| public List (String key, String value) { |
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.
Тут лишний пробел перед "("
| @Test | ||
| public void sizeChangesOnAdd() throws Exception { | ||
| List list = new List(); | ||
| assertEquals (0, list.size()); |
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.
И тут лишний пробел перед "("
No description provided.