Lab1#12
Conversation
Made all abstract classes + pullTaskGenerator + two specific tasks
Made all necessary classes and interfaces + some system exceptions
Created 5 Quisez and test system for them
All made tests are passed. All generators are nested in tasks.
| Map<String, Quiz> result = new TreeMap<>(); | ||
| ExpressionTask.Generator firstTaskGenerator = new ExpressionTask.Generator( | ||
| 1, 100, EnumSet.allOf(MathTask.Operation.class)); | ||
| Quiz firstQuiz = new Quiz(firstTaskGenerator, 10); |
There was a problem hiding this comment.
Все константы лучше выносить в отдельный файлик. Это касается про кол-во тестов и параметры генератора.
| TextTask thirdTextTask = new TextTask("Лектор расписался в журнале." + | ||
| " Там было 3 н-ки." + | ||
| " Сколько студентов было на паре?", "Один"); | ||
| TextTask foursTextTask = new TextTask("what the fastest mammal in the world?", "Cheetah"); |
There was a problem hiding this comment.
Помарка foursTextTask. В нескольких местах.
| secondTextTask, | ||
| thirdTextTask, | ||
| foursTextTask, | ||
| firstTextTask); |
| public Quiz(Task.Generator generator, int taskCount) { | ||
| this.generator = generator; | ||
| this.taskCount = taskCount; | ||
| nextTask = generator.generate(); |
There was a problem hiding this comment.
Тут лучше конструировать задания только по мере надобности в nextTask.
|
|
||
| public ExpressionTask(int firstNumber, int secondNumber, Operation operation) { | ||
| super(firstNumber, secondNumber, operation); | ||
| text = firstNumber + Operation.toChar(operation) + String.valueOf(secondNumber) + "=?"; |
There was a problem hiding this comment.
String.valueOf(firstNumber)
Без этого совсем не правильно генерируются задания.
There was a problem hiding this comment.
У меня сразу оба String.valueOf() были, когда я тестировал. Потом IDE сказало, что второй можно убрать, а я случайно первый убрал
| answer = "invalid operation"; | ||
| return; | ||
| } | ||
| answer = String.valueOf(calculate(firstNumber, secondNumber, operation)); |
There was a problem hiding this comment.
calculate возвращает int, а ты никак не проверяешь, что деление будет целочисленным. Это касается и EquationTask.
There was a problem hiding this comment.
Переделал, чтобы всё с даблами работало
| if (positionOfX == 0) { | ||
| text = "X" + Operation.toChar(operation) + firstNumber + "=" + secondNumber; | ||
| if (firstNumber == 0 && operation == Operation.DIVISION) { | ||
| answer = "invalid operation"; |
There was a problem hiding this comment.
На мой взгляд везде, в таких случаях, лучше было бы перегенерить (за это не снижал).
| return generators.get(i).generate(); | ||
| } | ||
| catch (Exception e) { | ||
| if (i == generators.size()) { |
There was a problem hiding this comment.
Тут лучше было бы просто continue. Не совсем понятно зачем нам ошибка какого-то генератора.
| */ | ||
| @Override | ||
| public Task generate() { | ||
| if (isAllowedDuplicate) { |
There was a problem hiding this comment.
Тут мне не очень нравится, зачем хранить еще один set usedTextsOfTasks. Если нам нужны дубликаты просто сохраняем в массив, если нет убираем их, а потом просто делаем shuffle.
| import java.util.EnumSet; | ||
|
|
||
| public class EquationTask extends AbstractMathTask { | ||
| int positionOfX; |
There was a problem hiding this comment.
Можно добавить модификатор доступа и константность. В других файлах тоже посмотри на это внимательно.
| } | ||
|
|
||
| default MathTask.Operation getRandomOperationFromSet() { | ||
| while (true) { |
There was a problem hiding this comment.
Подумай как это сделать элегантнее.
|
Ну пока что такая оценка:
В итоге 7.5 . В остальном хорошая работа. +2 балла за допы. |
|
Будет ещё один коммит минимум. Пока не перепроверять |
|
Исправил всё, кроме выноса констант. Также перевёл все вычисления в даблы и сделал доп на пресижен и пресижен в ответе. Можно перепроверять |
2 participants
Сделаны все обязательные пункты, кроме кастомных эксепшненов. Также сделан енам и нестед классы