Skip to content

Строганцев Антон#3

Open
blast-sky wants to merge 1 commit into
AleksandrPanov:masterfrom
blast-sky:master
Open

Строганцев Антон#3
blast-sky wants to merge 1 commit into
AleksandrPanov:masterfrom
blast-sky:master

Conversation

@blast-sky
Copy link
Copy Markdown

No description provided.

Comment thread src/Operations.hpp
Comment on lines +94 to +97
for (const auto& operation : _unarys)
if (std::strncmp(operation->getSymbols().c_str(), symbols.c_str(), operation->getSymbols().size()) == 0)
return operation.get();
return nullptr;
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.

можно использовать std::find
только нужно перегрузить оператор ==
можно классы IUnaryOperation, IBinaryOperation,... унаследовать от общего класса с перегруженным ==, равенство можно проверять через getSymbols
Правда вопрос, что делать с унарным и бинарным минусом (dynamic_cast?)

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.

Да, так можно было сделать, но две эти функции все равно нельзя было бы объединить в одну, т.к. для корректного анализа вводимых данных парсер ожидает в зависимости от предыдущего токена операцию конкретного типа (это как раз таки сделано во избежание конфликта операций разных типов, имеющих одно обозначение, как "-").

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.

тут проще добавить поле с уникальным id (для каждого нового класса) и делать проверку по нему

Comment thread src/Operations.hpp
Comment on lines +122 to +133
Operations()
{
_unarys.emplace_back(new UnaryMinus());
_unarys.emplace_back(new Sinus());
_unarys.emplace_back(new Cosinus());

_binarys.emplace_back(new BinaryPlus());
_binarys.emplace_back(new BinaryMinus());
_binarys.emplace_back(new Multiplication());
_binarys.emplace_back(new Division());
_binarys.emplace_back(new Pow());
}
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.

Да, это действительно стоило сделать.

Comment thread src/Operations.hpp
Comment on lines +119 to +120
TUnarys _unarys;
TBinarys _binarys;
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.

Казалось бы _unarys, _binarys можно сделать статическими, все остальные методы тоже. Тогда получается можно и без паттерна Singleton можно обойтись? Или нет? А если можно обойтись, то почему (и в каких случаях) это хуже?

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.

В принципе, любой шаблон проектирования имеет более "топорные" аналоги. В данном случае замена на статические списки и правда не дала бы заметных изменений, но существуют ситуации, например, где экземпляр класса имеет промежуточные состояния, для которых также бы пришлось заводить отдельные переменные и неиспользуемые пользователем (приватные) методы, что привело бы к загромождению рабочего пространства и возможным ошибкам использования сущности.

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

Labels

good first issue Good for newcomers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants