Виктор Сенин, Число с размерностью #8
Conversation
…ных тестов на работоспособность класса. Осталось сделать чтение из строки.
|
|
||
| private Map<String,Double> dimens; | ||
| { | ||
| dimens = new HashMap<>(); |
There was a problem hiding this comment.
Не обязательно использовать блоки инициализации. Можно просто после объявления переменной (на 17 строке) написать = new HashMap<>();
| return dimens; | ||
| } | ||
|
|
||
| Dimension getLengthDimensions() { |
There was a problem hiding this comment.
Подобные методы лучше сделать статическими. Ведь чтобы получить экземпляр Dimension с заполненными полями, нам не обязательно иметь другой экземпляр Dimension.
| try { | ||
| return dimens.get(dimension); | ||
| } | ||
| catch (NullPointerException e) { |
There was a problem hiding this comment.
Ловить NullPointerException - плохая идея. NullPointerException в большинстве случаев означает баг в коде, какую-то неисправность. Не стоит использовать это исключение в процессе обычной работы программы, это запутывает и снижает быстродействие. Проверяйте штатными способами, есть ли у вас такой элемент в Map-е, используйте сравнение с null, если хотите проверить корректность аргументов.
|
|
||
| import java.util.logging.Level; | ||
|
|
||
| /** |
There was a problem hiding this comment.
То, что пишете комментарии - это очень хорошо. Но лучше писать их непосредственно около функций, которые в описываете.
|
|
||
| @Override | ||
| public NumberWithDimension plus(NumberWithDimension other) { // Сложение размерных чисел | ||
| if(dimensionClass.getDimens().equals(other.dimensionClass.getDimens())) { |
There was a problem hiding this comment.
Используйте equals сразу применительно к dimensionClass.
There was a problem hiding this comment.
Кроме того, чтобы сократить уровни вложенности, лучше сразу сделать throw, когда размерности не совпадают.
| @Override | ||
| public double divideForDim(NumberWithDimension other) { // Деление на другое число с размерностью | ||
| if(dimensionClass.getDimens().equals(other.dimensionClass.getDimens())) { | ||
| return number / (other.number * (other.dimensionClass.getValueDimension(other.dimension) / |
| } | ||
|
|
||
| @Override | ||
| public NumberWithDimension translation (String other) { //Переводим вещественное число из одной величины в другую |
There was a problem hiding this comment.
Здесь есть пробел после имени метода, где-то его нет. Сделайте Reformat в IDEA.
|
|
||
| @Override | ||
| public int compareTo(NumberWithDimension other) { //Cравниваем два числа, подводя их под одну величину | ||
| return (number * dimensionClass.getValueDimension(dimension)) > (other.number |
There was a problem hiding this comment.
Сложный код, легко запутаться. Подумайте, как сделать его понятнее.
|
|
||
| String getDimension(); // Взятие размерности из класса в строковой форме | ||
|
|
||
| NumberWithDimension plus(NumberWithDimension other); // Сложение двух вещественных чисел с размерностью |
| } | ||
|
|
||
| @Test | ||
| void minus() { |
There was a problem hiding this comment.
Сделайте код поаккуратнее, тяжело читать. А так вроде нормальные тесты. Только не хватает проверок некорректных ситуаций - в коде есть много throw, а здесь вы это никак не проверяете.
h31
left a comment
h31
left a comment
There was a problem hiding this comment.
Оценка "Отлично". Если исправите оставшиеся замечания - будет вам плюсик в карму.
| */ | ||
| public class Dimensions { | ||
|
|
||
| Dimensions(){ |
| /** | ||
| * Дублирующий метод для вычисления чисел с размерностью. Используется во многих дальнейших функциях. | ||
| */ | ||
| private double calculation(NumberWithDimension other) { |
There was a problem hiding this comment.
По названию метода непонятно, зачем он нужен. Подберите более подходящее название. Если это вычисления, то чего именно?
| */ | ||
| @Override | ||
| public NumberWithDimension minus(NumberWithDimension other) { | ||
| return new NumberWithDimension(number - calculation(other), dimension, dimensionClass); |
There was a problem hiding this comment.
Мелочь, но всё же есть некоторое дублирование между этим и двумя следующими методами.
| * Деление двух вещественных чисел друг на друга. В результате получаем вещественное число. | ||
| */ | ||
| @Override | ||
| public double divideForDim(NumberWithDimension other) { |
There was a problem hiding this comment.
Этот метод также можно назвать divide. Методы могут иметь одинаковые названия, если у них разные названия (method overloading).
| * Перевод вещественного числа с размерностью из одной размерности в другую. | ||
| */ | ||
| @Override | ||
| public NumberWithDimension translate(String other) { |
| .addDimension("А",1) | ||
| .addDimension("мА",0.001)))); | ||
| logger.info("Начало теста на деление на вещественное число с размерностью, чтобы мы ловили ошибку."); | ||
| try {assertEquals(new NumberWithDimension(4.0,"км", |
There was a problem hiding this comment.
В JUnit 5 есть прекрасный метод для тестирования исключений. Он позволит сделать ваш код более читаемым. Касается и другого кода, который тестирует исключения.
| void plus() { | ||
| logger.info("Начало теста на сложение с вещественным числом с размерностью, " + | ||
| "используя уже приготовленные размерности."); | ||
| assertEquals(new NumberWithDimension(4.0,"км", |
There was a problem hiding this comment.
Подобная "лесенка" затрудняет чтение. Постарайтесь сделать, чтобы не было таких разных отступов. Например, вам в этом помогут временные переменные.
| assertEquals(new NumberWithDimension(4.0,"км", | ||
| new Dimensions().getLengthDimensions()), | ||
| new NumberWithDimension(2.0,"км", | ||
| new Dimensions().getLengthDimensions()) |
There was a problem hiding this comment.
Dimensions.getLengthDimensions(). Это же статический метод.
| System.out.println("Hello World!"); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Внезапно откуда-то появившиеся пустые строки.
3 participants
No description provided.