HW5 #7
HW5 #7
Conversation
yurii-litvinov
left a comment
yurii-litvinov
left a comment
There was a problem hiding this comment.
Папка .gradle не нужна, .idea/workspace.xml тоже не нужен
| * Binary search tree working as a set, capable of adding/checking elements. | ||
| * @param <T> type of the object it stores | ||
| */ | ||
| public class BinaryTree<T> { |
| /** | ||
| * The binary tree subclass representing the node of a tree. | ||
| */ | ||
| class Node { |
There was a problem hiding this comment.
А точно нужна пакетная видимость?
| class Node { | ||
| T value; | ||
| Node left; | ||
| Node right; |
There was a problem hiding this comment.
Пакетная видимость у поля уж точно не нужна, если не хоитте, чтобы баллы за задачу ушли в минус :)
| BinaryTree<Integer> binaryTree = new BinaryTree<>(); | ||
| binaryTree.add(0); | ||
| binaryTree.add(10); | ||
| assert (binaryTree.size() == 2); |
There was a problem hiding this comment.
assertEquals, assert --- это не для тестов, а для проверки инвариантов во время выполнения
|
|
||
| public class BinaryTreeTest { | ||
| @Test | ||
| public void add() throws Exception { |
There was a problem hiding this comment.
Нужны более адекватные имена тестов
| for (Maybe<Integer> maybeInt : maybeArray) { | ||
| String toWrite; | ||
| try { | ||
| Integer result = maybeInt.map(value -> value*value).get(); |
There was a problem hiding this comment.
Бинарные арифметические операции выделяются пробелами всегда
| try { | ||
| Integer result = maybeInt.map(value -> value*value).get(); | ||
| toWrite = Integer.toString(result); | ||
| } catch (NothingException e) { |
There was a problem hiding this comment.
Плохая идея управлять исполнением программы с помощью исключений, исключения должны бросаться только в неожиданных ситуациях, а тут мы по условию знаем, что часть Maybe будет Maybe.nothing()
| @@ -0,0 +1,2 @@ | |||
| public class NothingException extends Exception { | |||
There was a problem hiding this comment.
Исключению тоже нужен комментарий
| } catch (NothingException ex) { | ||
| exceptionThrown = true; | ||
| } | ||
| assert (exceptionThrown); |
There was a problem hiding this comment.
Ужасно :) Для этого ведь есть параметр expected у @Test
|
|
||
| public class MaybeTest { | ||
| @Test | ||
| public void get() throws Exception { |
There was a problem hiding this comment.
Нужны более содержателдьные комментарии к тестам
yurii-litvinov
left a comment
yurii-litvinov
left a comment
There was a problem hiding this comment.
Дерево зачтено, Maybe надо доправить
| * @param value object to add | ||
| * @return true if there was this object already, false otherwise | ||
| */ | ||
| public boolean add (T value) { |
There was a problem hiding this comment.
Пробел перед "(" лишний, тут и ниже
| @@ -0,0 +1,76 @@ | |||
| package ru.spbau.maybe; | |||
|
|
|||
| import com.sun.istack.internal.NotNull; | |||
There was a problem hiding this comment.
Я думаю, имелась в виду org.jetbrains.annotations.NotNull, а то в Java 9 такого нет
| } | ||
|
|
||
| dependencies { | ||
| testCompile group: 'junit', name: 'junit', version: '4.12' |
There was a problem hiding this comment.
Если хотите @NotNull, то надо зависимость от org.jetbrains.annotations тоже добавить
| */ | ||
| private T value; | ||
|
|
||
| private Maybe (T t) { |
| * @return Maybe containing null | ||
| */ | ||
| public static <T> Maybe<T> nothing() { | ||
| return nothing; |
There was a problem hiding this comment.
Имеет смысл добавить методу @SuppressWarnings("unchecked"), это же сырое значение
| assert (mbString2.get().equals("test")); | ||
| } catch (NothingException ex) { | ||
| assert (false); | ||
| } |
There was a problem hiding this comment.
Вот ещё недобитые юнит-тесты :)
yurii-litvinov
left a comment
yurii-litvinov
left a comment
There was a problem hiding this comment.
Теперь всё ок, зачтена
No description provided.