Skip to content

Lab 1: Quizer#29

Open
DudkoAndrei wants to merge 24 commits into
Rrenkens:mainfrom
DudkoAndrei:quizer
Open

Lab 1: Quizer#29
DudkoAndrei wants to merge 24 commits into
Rrenkens:mainfrom
DudkoAndrei:quizer

Conversation

@DudkoAndrei
Copy link
Copy Markdown

@DudkoAndrei DudkoAndrei commented Nov 29, 2022

Сделаны все допы, кроме чтения из json-ов

quizer

this.answer = answer;
}

@Override
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Вот этот метод можно было объединить с MathTask и сделать какой-то абстрктный класс.

* исключение, то и тут выбрасывается исключение.
*/
public Task generate() {
Collections.shuffle(generators);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Можно было 1 раз зашафлить и ходить по кругу. Shuffle дорогая операция и явно здесь не требуется больше одного раза.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Исправил

this.allowDuplicate = allowDuplicate;
this.tasks = new ArrayList<>(tasks);

if (!allowDuplicate) {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Надо еще проверить, что в изначальных задачах нету дубликатов.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Исправил


private Task currentTask = null;
private final int taskCount;
private boolean answeredOnCurrentTask = false;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Мне кажется, что лучше хранить последний результат.

}

double getRandomDouble() {
return truncate(rnd.nextDouble(minNumber, maxNumber + Double.MIN_VALUE), precision);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

А зачем здесь Double.MIN_VALUE?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Исправил

super(operation.compute(firstNumber, secondNumber), precision);

df.setMaximumFractionDigits(precision);
df.setMinimumFractionDigits(precision);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

boolean isFirstNumberUnknown = rnd.nextBoolean();
Operation operation = getRandomOperation();

if (operation == Operation.DIVIDE && isDoubleEqual(secondNumber, 0.0, eps)) {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Вот 0 / x = 0 релевантный пример.

if (operation == Operation.DIVIDE && isDoubleEqual(secondNumber, 0.0, eps)) {
operation = Operation.MULTIPLY;
}
if (operation == Operation.MULTIPLY && isDoubleEqual(firstNumber, 0.0, eps)) {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

а тут чем 0.0 * x = 0.0 не подойдет?

@Rrenkens
Copy link
Copy Markdown
Owner

Rrenkens commented Dec 7, 2022

Ну пока что такая оценка:

  • -0.15 за проверку дубликатов.
  • -0.6 за нарушение инвариантов знаков. Допустим, я выставил, что возможная операция только деление, затем генерируется 0, и по-твоему коду получаю какую-то другую операцию. И еще ты отсеиваешь релевантные примеры. Кейсы довольно редкие, поэтому не сильно снижаю.

Допы:

  • +1 за EnumSet.
  • +1 за nested class.
  • +3 за precision во всех его проявлениях.
  • +0 за UML. Это диаграмма классов, а нужно показать их взаимодействие между собой.

В итоге 9.25. В остальном супер-пупер. +5 за допы.

@Rrenkens Rrenkens added the question Further information is requested label Dec 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

question Further information is requested

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants