Skip to content

Реализация игры#1

Open
anzotov wants to merge 12 commits into
mainfrom
devel
Open

Реализация игры#1
anzotov wants to merge 12 commits into
mainfrom
devel

Conversation

@anzotov

@anzotov anzotov commented Mar 24, 2024

Copy link
Copy Markdown
Owner

No description provided.

Comment thread WidgetMovement/mainwindow.cpp Outdated

m_timer = new QTimer(this);
connect(m_timer, &QTimer::timeout, this, [this]() {
this->m_timer->setInterval(QRandomGenerator::global()->bounded(100, 1000));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this писать не нужно, если переменные не экранированы. В данном случае это лишнее. Захват this в лямбду позволяет опускать this->, как будто мы находимся внутри метода.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

В прошлой домашке так и писал, а тут что-то подзабыл :)

@Toxin65 Toxin65 Mar 25, 2024

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Кстати. Как говорит мой коллега: "Не надо делать это членом." m_timer в принципе можно не хранить полем окна, он же управляется из лямбд, куда он захвачен. Но я не против, это просто NIT.

Comment thread WidgetMovement/mainwindow.cpp Outdated

auto button = new QPushButton(this);
constexpr auto buttonSize = QSize(15, 15);
auto buttonPos = QPoint(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

const облегчит чтение. Мы будем понимать, что ниже это значение уже не будет меняться.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

То есть надо все переменные по возможности обмазывать const'ами, даже если значение используется уже на следующей строчке?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

По хорошему да. Я обычно говорю что надо const писать везде, где можно написать const. Кроме тех мест, где const написать нельзя.
Потому что const -- это ещё один предохранитель на вашем оружии. У вас и у ваших коллег будет меньше шансов отстрелить себе конечность. Кто-то говорит "зачем это надо, тут же всё очевидно", а потом может быть случайная модификация, которую сложно найти визуально.

Comment thread WidgetMovement/mainwindow.cpp Outdated
Comment thread WidgetMovement/mainwindow.cpp Outdated
button->deleteLater();});

buttonTimer->start(QRandomGenerator::global()->bounded(15, 30)); });
m_timer->start(1000);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Нарушение требования ТЗ. Период появления кнопок тоже произвольный.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Поправить, конечно, не сложно, но не совсем согласен, что это нарушение ТЗ. В ТЗ сказано:

Интервал времени между появлением объектов случайный от 0.1 до 1 секунды

А здесь задается время между запуском программы и появлением первого объекта, которое в ТЗ не задано.

@Toxin65 Toxin65 Mar 25, 2024

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Извиняюсь, я проглядел у вас строчку на этот предмет...
m_timer->setInterval(QRandomGenerator::global()->bounded(minCreationTimeMs, maxCreationTimeMs));

Поэтому всё было корректно сразу.

@Toxin65 Toxin65 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Выявлены несоответствия требованиям ТЗ.

@anzotov anzotov requested a review from Toxin65 March 25, 2024 15:25

m_timer = new QTimer(this);
constexpr auto minCreationTimeMs = 100, maxCreationTimeMs = 1000;
connect(m_timer, &QTimer::timeout, this, [this, minCreationTimeMs, maxCreationTimeMs]() {

@Toxin65 Toxin65 Mar 25, 2024

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Уверено, что minCreationTimeMs и maxCreationTimeMs не надо захватывать в лямбду. Попробуйте. И подумайте, почему так.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Да, не знал о такой особенности. Спасибо!

QRandomGenerator::global()->bounded(0, 100 - buttonSize.height())
);
button->setGeometry(QRect(buttonPos, buttonSize));
button->setText("*");

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Это можно сделать в конструкторе кнопки.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Хм, действительно, но что-то я логики в Qt не уловил. В конструкторе QPushButton можно задать текст, но свойства text у объекта класса QPushButton нет, это свойство его базового класса QAbstractButton. Зато у класса QAbstractButton есть свойство text, но задать его в конструкторе почему-то нельзя. Чудеса!

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Не, нормально. Почему нет?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Ладно, видимо сделали так, как удобно программистам. Постараюсь внимательнее разглядывать конструкторы в следующий раз, а то привык только parent писать, а там, оказывается, еще что-то бывает. Спасибо!

});

connect(button, &QPushButton::pressed, button, [button](){
button->deleteLater();});

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Тут лямбда не нужна. Можно обращаться как в слот.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Точно, не сообразил.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants