Замечания по Vector 3d

[alexey-malov]

• [x] В бинарных арифметических операторах лучше возвращать const CVectro3D, чтобы обезопасить себя от случайного присваивания результату операции вместо сравнения:

  auto x = (v1 + v2) = v3;

[alexey-malov]

    friend CVector3D operator* (CVector3D const & vector3d, double scalar);
    friend CVector3D operator* (double scalar, CVector3D const & vector3d);

• [x] В объявлении операции дружественными нет необходимости (можно просто объявить вне класса эти операторы), т.к. приватных полей в CVector3D, к которым был бы необходимо доступ, нет

[alexey-malov]

• [ ] Операторы +=, -=, *=, /= должны возвращать ссылку на CVector3D

if ((v1 += v2) == v3)
{
}

[alexey-malov]

• [x] Не реализовано скалярное произведение векторов

• [x] Не реализовано векторное произведение векторов

• [x] Не реализована нормализация вектора

[alexey-malov]

CVector3D::CVector3D()
{
    x = y = z = 0.0;
}

CVector3D::CVector3D(double x0, double y0, double z0)
{
    x = x0;
    y = y0;
    z = z0;
}

• [x] Используйте списки инициализации конструктора для задания начальных значений полей

[alexey-malov]

CVector3D CVector3D::operator+ (CVector3D const & vector1) const
{
    CVector3D result;
    result.x = x + vector1.x;
    result.y = y + vector1.y;
    result.z = z + vector1.z;
    return result;
}

• [x] Лучше передать в конструктор результирующего вектора координаты, чем перезаписывать их после обнуления. Можно вообще написать:

  return { x + vector1.x, y + vector1.y, z + vector1.z };

Это касается не только сложения, но и других арифметических операций

[alexey-malov]

CVector3D CVector3D::CVector3D::operator- () const
{
    CVector3D result;
    return *this * -1;
}

• [x] result не используется. Лучше написать return {-x, -y, -z};

[alexey-malov]

• [x] В тестах операторово сравнения аргументы лучше объявить прямо внутри самого теста, чтобы не приходилось переключаться на далекий участок кода

BOOST_CHECK(CVector(1, 2, 3) == CVector(1, 2, 3));

• [ ] В тестах операторов == и != должно быть как минимум 4 проверки (в каждом). Причем, все координаты векторов должны быть разными.

[alexey-malov]

    BOOST_AUTO_TEST_CASE(return_false_when_not_all_fields_of_both_vectors_are_equal)
    {
        CVector3D vector3(vector1);
        vector3.x++;
        BOOST_CHECK(!(vector3 == vector1));
    }

• [x] Икремент для double лучше не применять

[alexey-malov]

• [x] В тестах GetLength() следует входные данные поставлять такими, чтобы результат использовал все компоненты вектора.

[alexey-malov]

• [ ] Не реализованы операторы ввода-вывода

[alexey-malov]

const CVector3D CrossProduct(CVector3D const& vector1, CVector3D const& vector2)
{
    return{ vector1.y * vector2.z - 
        vector1.z * vector2.y, vector1.z * vector2.x - 
        vector1.x * vector2.z, vector1.x * vector2.y - 
        vector1.y * vector2.x };
}

• [ ] отформатировать код для лучшего чтения.

[alexey-malov]

const CVector3D CVector3D::Normalize()
{
    double length = GetLength();
    if (length != 0)
    {
        x /= length;
        y /= length;
        z /= length;
    }
    return *this;
}

• [ ] Либо возвращать void и модифицировать себя, либо возвращать новый CVector3d, а себя не модифицировать

• [ ] Метод не покрыт тестами

[alexey-malov]

    BOOST_AUTO_TEST_CASE(return_false_when_y_of_both_vectors_not_are_equal)
    {
        CVector3D vector1 = { 1, 2, 3 };
        CVector3D vector3 = { 1, 5, 3 };
        BOOST_CHECK(!(vector3 == vector1));
        BOOST_CHECK(vector3.x == vector1.x); //!
        BOOST_CHECK(vector3.y != vector1.y); //!
        BOOST_CHECK(vector3.z == vector1.z); //!
    }

• [ ] Отмеченные ! проверки уже проверялись в конструкторе. Лучше сделать один тест для каждого оператора сравнения, содержащий 4 варианта сравнения