Skip to content

YuriyKuznetsov_HomeWork8#54

Open
GKuzzinshtern wants to merge 1 commit intomainfrom
YuriyKuznetsov_HomeWork8
Open

YuriyKuznetsov_HomeWork8#54
GKuzzinshtern wants to merge 1 commit intomainfrom
YuriyKuznetsov_HomeWork8

Conversation

@GKuzzinshtern
Copy link
Collaborator

No description provided.

double CalcSquare() override;

void Side(const unsigned int in_uiSide) { uiSide__ = in_uiSide; }
unsigned int Side() { return uiSide__; }
Copy link
Collaborator

Choose a reason for hiding this comment

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

eсли метод ничего не меняет, может стоит сделать его const

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ага. Надо завести эту привычку :)

RectSides &operator=(const RectSides &in_Sides) {
uiSideOne = in_Sides.uiSideOne;
uiSideTwo = in_Sides.uiSideTwo;
return *this;
Copy link
Collaborator

Choose a reason for hiding this comment

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

а для чего тут перегрузка присваивания? ведь структуры одного типа и так можно присваивать друг другу)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

больше, чтоб поюзать перегрузку операторов.

RectSides Sides() { return Sides__; }

private:
RectSides Sides__{};
Copy link
Collaborator

Choose a reason for hiding this comment

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

это просто мнение, но по моему тут не нужна структура, ведь мы и так инкапсулируем поля в классе. А так матрешка получается

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

конечно, можно обойтись и без структуры. Мне понравилась идея передавать все стороны как объект.

}

double Square::CalcSquare() {
double dArea{static_cast<double>(uiSide__) *
Copy link
Collaborator

Choose a reason for hiding this comment

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

а для чего тут cast к double и возвращаемое значение double? ведь сторона квадрата - целое число. Можно возвращать long long

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

в случаях других фигур результат может быть дробным. Поэтому в базовом классе был принят double, а следовательно и в наследниках.

#include <cmath>
#include <iostream>

class Shape {
Copy link
Collaborator

Choose a reason for hiding this comment

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

если ошибаюсь, Николай меня поправит, но если мы собираемся работать через указатель на базовый класс и виртуальные функции, то для корректного разрушения объеков в базовом классе нужно обьявить виртуалный деструктор.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

да, наверное стоит его написать, хотя бы для того, чтоб не забыть впоследствии. В моем случае нет захвата ресурсов, поэтому я деструктор не указал.

@@ -0,0 +1,141 @@
// Реализовать переопределение метода базового класса в
//производных классах применив виртуальные функциии
Copy link
Collaborator

Choose a reason for hiding this comment

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

Добрый вечер! по многочисленным просьбам решил опробовать себя в новой роли) поэтому за качество ревью прошу понять и простить)

Copy link
Collaborator 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