Базовый Класс C++ Круг


Я пытаюсь улучшить мои базовые навыки C++ и создать действительно красивый, лаконичный и чистый код, свободный от артефактов.

Программа может складывать и вычитать значения радиуса каждого круга, назначить одного круга к другому, и кроме того дают.

Любые советы о том, как эта программа может быть улучшена, то ли рассматривая вопрос читабельности или что-нибудь еще, было бы здорово.

//  Basic circle exercise 23rd March

#include <cstdio>
#include <math.h>

using namespace std;

//==============================================================================
/*                               interface                                    */
//==============================================================================
namespace corey
{
    class Circle
    {
    public:
        Circle(const int & r = 1.0);
        ~Circle();

        const double & getRadius() const;
        const double getArea() const;

        Circle & operator = (const Circle &);
    private:
        double radius;
    };
};


//==============================================================================
/*                              implementation                                */
//==============================================================================
/*I'd probably included using namespace corey if
    i'd have bothered with seperate files*/

corey::Circle::Circle(const int & r) : radius(r){}

corey::Circle::~Circle(){}

const double & corey::Circle::getRadius() const
{
    return radius;
}

const double corey::Circle::getArea() const
{
    const double area = (M_PI * radius * radius);
    return area;
}

corey::Circle operator + ( const corey::Circle & lhs, const corey::Circle & rhs)
{
    return corey::Circle(lhs.getRadius() + rhs.getRadius());
}

corey::Circle operator - ( const corey::Circle & lhs, const corey::Circle & rhs)
{
    return corey::Circle(lhs.getRadius() - rhs.getRadius());
}

corey::Circle & corey::Circle::operator = (const Circle & lhs)
{
    if(&lhs != this)
    {
        radius = lhs.radius;
    }
    return (*this);
}


//==============================================================================
/*                                   useage                                   */
//==============================================================================
int main( int argc, char ** argv )
{
    //this stuff is just some info printed out so not super important...
    corey::Circle c1(2);
    printf("Circle made with radius %lf & area %lf\n", c1.getRadius(), c1.getArea());

    const corey::Circle c2(6);
    printf("Circle made with radius %lf & area %lf\n", c2.getRadius(), c2.getArea());

    corey::Circle c3;
    c3 = c2 + c1;
    printf("Circle made with radius %lf & area %lf\n", c3.getRadius(), c3.getArea());

    c1 = c3;
    printf("Circle made with radius %lf & area %lf\n", c1.getRadius(), c1.getArea());

    return 0;
}


2968
6
задан 23 марта 2018 в 12:03 Источник Поделиться
Комментарии
7 ответов

Я думаю, это хорошее начало. Это простой и легко читаемый. Данные неизменны, что делает его менее подвержены ошибкам. Вот некоторые вещи, которые я бы изменил.

Это Полный?

Я заметил, что ваш круг не имеет место. Почему нет? В большинстве код, который я имел дело с тем, где у меня есть форма, там почти всегда место или должность, где круг в пространстве. Обычно это необходимо, потому что мне часто нужно рисовать его, пересекающая ее, нажмите тест против нее и т. д. Я хотел бы предложить создание Point или Coordinate класса и используя объект класса в качестве места нахождения Circle.

Осторожнее С Перегрузками

Возможность перегружать основные математические операторы-это приятная особенность c++. Однако, это может быть запутанным, если смысл этих перегрузок не очевидно, глядя на определение класса. Это не сразу очевидно для меня, что значит добавить 2 круга. Есть места, где добавляя и вычитая формы смысл (конструктивной твердотельной геометрии), но результаты сильно отличаются от того, что происходит здесь. Я думаю, что перегрузки у вас, скорее всего, чтобы запутать будущих читателей кода. Я хотел бы сделать обычными методами с такими именами, как addRadiuses() и subtractRadiuses() вместо.

Избежать using namespace std

Говорят часто, что следует избегать использования using namespace std. Я позволю эксперты объясняют это.

Я также не думаю, что называя namespace после себя-это тоже полезно. Имя пространства имен должен сказать, что это и намек на то, что он может содержать. Я не знаю, что corey должен содержать.

14
ответ дан 23 марта 2018 в 02:03 Источник Поделиться

Быть последовательным с #include

Мы имеем c++ <cstdio> (который определяет с именами std пространства имен), но в стиле c <math.h> (что ставит идентификаторы в глобальном пространстве имен). Для нового C++ код, предпочитают <cmath>. И не использовать using namespace std; - что может быть вредным.

Позволить компилятору определить назначение

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

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

Передаче по значению для примитивных типов

Нет никакой выгоды для передачи ссылки на const int в конструктор - просто передать int по значению. Это и проще (до оптимизации) быстрее.

На самом деле, почему это int, когда мы могли бы принять любое double значение? И почему это по умолчанию 1.0? Я думаю, что 0.0 (пустой круг) было бы менее удивительно (это тогда больше похоже на цифры, где по умолчанию инициализации использует ноль). Рассмотрим также некоторые проверки - отрицательный радиус значимых для кода?

Лучше вернуться double по стоимости от getRadius() по тем же причинам.

Марк конструктор explicit

Если вы не любите сюрпризы, лучше, если вы не позволяете цифры преобразовать автоматически в кругах.

Не вернет константные значения

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

Операторы

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

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

На позитиве, у вас есть правильное аргументов и возвращаемых типов здесь.

Удобоносимость

Ваше окружение явно предоставляет вам M_PI за значение π. Это не стандартный идентификатор, и хотя это обычное расширение, это более портативный, чтобы определить свои собственные. Популярные выражения производстве π включают 4 * std::atan(1) и std::acos(-1) обе эти константы времени компиляции, так что ничего не стоило программы пользователей.

Мне нравится в getArea() это то, что вы пишете radius * radius и не соблазниться std::pow(radius, 2) - что это, как правило, медленнее и менее точный, чем умножение (это более универсальный, потому что он поддерживает дробные степени, но нам не нужны здесь).

Предпочитаю на C++ форматированный вывод

В <iostream> заголовок содержит ввода/вывода с Лучше типа проверки, чем в стиле c <cstdio> функции. Рассмотреть вопрос о предоставлении << оператор на поток в стандартной библиотеке потока.


Измененный код

#include <cmath>
#include <iosfwd>

namespace corey
{
class Circle
{
static constexpr double pi = 4 * std::atan(1);
double radius;

public:
explicit Circle(double r = 0);

double getRadius() const;
double getArea() const;
};

Circle operator+(const Circle&, const Circle&);
Circle operator-(const Circle&, const Circle&);

std::ostream& operator<<(std::ostream&, const Circle&);
}

#include <ostream>

namespace corey {
Circle::Circle(double r)
: radius(r)
{}

double Circle::getRadius() const
{
return radius;
}

double Circle::getArea() const
{
return pi * radius * radius;
}

Circle operator+(const Circle& lhs, const Circle& rhs)
{
return Circle(lhs.getRadius() + rhs.getRadius());
}

Circle operator-(const Circle& lhs, const Circle& rhs)
{
return Circle(lhs.getRadius() - rhs.getRadius());
}

std::ostream& operator<<(std::ostream& os, const Circle& c)
{
return os << "Circle made with radius " << c.getRadius()
<< " & area " << c.getArea();
}
}

#include <iostream>

int main()
{
using corey::Circle;

Circle c1(2);
std::cout << c1 << '\n';

const Circle c2(6);
std::cout << c2 << '\n';

Circle c3;
c3 = c2 + c1;
std::cout << c3 << '\n';

c1 = c3;
std::cout << c1 << '\n';
}

20
ответ дан 23 марта 2018 в 09:03 Источник Поделиться

Я присоединяюсь к тому, что @user1118321 и @документальные правовые акты сказал, но хочу добавить несколько замечаний.

О ссылках

Ссылки не всегда путь. В вашем конструкторе (Circle(const int & r = 1.0);), Он не является оптимальным для получения аргумента по ссылке, потому что int будет меньше, чем ссылка (указатель под капотом), как правило, 4 байта против 8 байт. Так что будет лучше передать его по значению: Circle(int r = 1). Кстати, 1.0 это float, Так что задание выглядит немного странно.

Идем, как возвращаемое значение: const double & corey::Circle::getRadius() const. Ссылку будет выступать в качестве ценности, поэтому возврат по значению лучше (и более идиоматические): double Circle::getRadius() const.

Об инкапсуляции

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

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

Вам лучше начинать с того, что функциональность, которую вы хотите предложить, и получают своей реализации от него. Если ваш круг определяется только ее радиусом, это в основном double. И я могу подвести итог весь код на:

using Circle = double; 
double area(Circle radius) { return M_PI * radius * radius; }
// I get all the rest for free since +, =, etc. are already defined for double

О константность

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

Вы можете это сделать, реализует две функции-члена:

double  radius() const { return radius; }
double& radius() { return radius; } // will be called if `this` isn't const

Или, вы можете упростить более радикально свой дизайн и реализовать Circle как struct: struct Circle { double radius; }; кажется, достаточно хороший, по крайней мере, пока у вас есть хорошая причина, чтобы не быть доволен.

О случае

Нет жестких правил, но get_radius более C++-как и чем getRadius

8
ответ дан 23 марта 2018 в 09:03 Источник Поделиться

Значение по умолчанию для конструктора.

Почему это производить круга радиуса 1 по умолчанию, если конструктор не имеет параметров? Если пользователь не указал радиус, они не должны получить результат, верно?

Лучшие практики здесь можно было бы сделать конструктор по умолчанию (без параметров) частная. Если вызывающий код не установить радиус, он просто не будет компилироваться.

Нет конструктора копирования.

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

getRadius возвращаемый тип.

Почему это возвращает ссылку на двойное, а не просто двойник?

Документации.

Я знаю, ты только начинаешь в C++, а это просто пробный урок бежать, но это хорошая практика, чтобы документировать интерфейс класса (и внутренних методов/данных при необходимости). Я предлагаю вам посмотреть в программе Doxygen, которая, как правило, лучшее-практиковать способ документирования кода C и C++.

Старайтесь не использовать C-стиля студии на C++.

Это не большое дело, но c++ стиль обычно предпочитают потокового ввода/вывода. std::cout вообще предпочитает printf.

5
ответ дан 23 марта 2018 в 02:03 Источник Поделиться

Я практиковался в C++, поэтому не буду комментировать код конструкции. Но я повторю то, что user1118321 говорил про то, что "полный". Я создал свой собственный Circle класс в VB.Net для геопространственного проекта. Эти методы и функции.

Public Class Circle
Private p_radius_sq As Decimal
Private p_Centre As PointD 'Double - based user type for coordinates
Private p_valid As Boolean ' if the construction fails for geometric reasons, the circle is invalid
Private p_geometryhelper As New Geometry 'my own class of useful geometry functions
Private p_rect As RectangleF

' Create circle from a point and a radius
Public Sub New(Centre As PointD, radius As Decimal)

' Create a circle from three points
Public Sub New(a As PointD, b As PointD, c As PointD)

' Creates a minimum bounding circle for a list of points.
Public Sub New(ThePoints As List(Of PointD))

' Useful property for real-world calculations
Public Property Radius_Squared() As Double

Public Property Centre() As PointD

Public ReadOnly Property Radius() As Double

Public Property Valid() As Boolean

' Identify where one circle intersects with another.
Public Function IntersectionPoints(OtherCircle As Circle) As List(Of PointD)

' Identifies if a set of points is enclosed by this circle
Private Function EnclosesPoints(points As List(Of PointD), Optional skip1 As Integer = -1, Optional skip2 As Integer = -1, Optional skip3 As Integer = -1) As Boolean

' See if the circle encloses the nominated point
Public Function EnclosesPoint(ThePt As PointD) As Boolean

End Class

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

4
ответ дан 23 марта 2018 в 05:03 Источник Поделиться

Поставить код реализации тоже внутри пространства имен

Вместо использования using директивы, или замусоривание кода с corey::С лучшая вещь, чтобы сделать, это приложить все осуществления внутри одного namespaceнапример,

namespace corey {
Circle::Circle() { ... }
}

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

4
ответ дан 23 марта 2018 в 10:03 Источник Поделиться

Если вы изучаете C++, то начинать нужно сразу с помощью квалификаторов, так что она становится второй натурой. Есть 3 заметных в вашем случае:


  1. const
    Вы уже используете это здорово

  2. noexcept
    Это говорит компилятору, что эта функция не может бросить. Следовательно, компилятор может упростить обработку исключений вокруг этой функции. Есть 6 функций для вашего цикла, из которых только 2 гарантированно noexept. Можете ли вы сказать, что и почему?

  3. constexpr
    Это относится к компиляции оценки времени. Это достаточно сложная тема, но вы обязательно должны прочесть как можно скорее, как возглавил в C++, чтобы идти как можно дальше на таких.

    Ваш конструктор может быть пользователем. Это превращает ваш круг класса в literal.
    Вы также можете применять пользователем все другие функции, кроме оператора поток. Пожалуйста, проинформируйте об отношении между noexcept и constexpr.


  4. Для полноты картины есть также следующие дополнительные квалификаторы, которые всегда должны быть использованы, когда это применимо: final и overload. Однако они имеют значение только в отношении виртуальных функций.

2
ответ дан 18 декабря 2018 в 01:12 Источник Поделиться