pimathvector_tests #38

Closed
andrey wants to merge 14 commits from pimathvector_tests into master
Owner
No description provided.
andrey requested review from zzuummaa 2020-09-23 10:58:10 +03:00
andrey reviewed 2020-09-23 11:07:43 +03:00
andrey left a comment
Author
Owner

нужно поправить пару моментов

нужно поправить пару моментов
@@ -70,1 +245,4 @@
* @param index is the index of necessary element
* @return element of vector
*/
Type operator [](uint index) const {return c[index];}
Author
Owner

здесь бь надо поменять на const Type &

здесь бь надо поменять на `const Type & `
@@ -197,1 +770,4 @@
* @param index is the index of necessary element
* @return element of vector
*/
Type operator [](uint index) const {return c[index];}
Author
Owner

здесь бь надо поменять на const Type &

здесь бь надо поменять на const Type &
@@ -200,1 +794,4 @@
* @param v vector for the compare
* @return if vectors are equal true, else false
*/
bool operator ==(const _CVector & v) const {PIMV_FOR(i, 0) if (c[i] != v[i]) return false; return true;}
Author
Owner

здесь необходимо дописать проверку размеров

здесь необходимо дописать проверку размеров
Author
Owner

кстати тут гораздо проще, можно написать вместо всего return c == v.c;

кстати тут гораздо проще, можно написать вместо всего `return c == v.c;`
@@ -214,0 +890,4 @@
* @param v is vector for cross product
* @return the result vector equal of cross product
*/
_CVector operator *(const _CVector & v) const {if ((c.size() != 3) && (v.size() != 3)) return _CVector(); _CVector tv(3); tv.fill(Type(1)); tv[0] = c[1]*v[2] - v[1]*c[2]; tv[1] = v[0]*c[2] - c[0]*v[2]; tv[2] = c[0]*v[1] - v[0]*c[1]; return tv;}
Author
Owner

в условии должно быть || а не &&
странно что тесты этого не показали (наверное тесты не охватывают все случаи)

в условии должно быть || а не && странно что тесты этого не показали (наверное тесты не охватывают все случаи)
andrey added a new dependency 2020-09-23 11:17:11 +03:00
Contributor

Просьба описывать в документации логику возвращения this более точно. Например метод PIMathVectorT::fill():

/**
* @brief Method that fills a vector with a value
*
* @param v value of which the vector is filled
* @return vector of type PIMathVectorT filled with "v"
*/
_CVector & fill(const Type & v) {PIMV_FOR(i, 0) c[i] = v; return *this;}

В блоке @return описание слишком расплывчато и не отражает того факта, что возвращаемый вектор- это тот же самый вектор, у которого был вызван метод fill().

Необходимо уточнить в документации логику возвращаемого значения для классов PIMathMatrixT, PIMathMatrix, PIMathVectorT, PIMathVector

Просьба описывать в документации логику возвращения `this` более точно. Например метод [PIMathVectorT::fill()](https://git.shs.tools/SHS/pip/src/commit/b7f035178f62ca6f3d7042cc39ebc9711904f6ac/libs/main/math/pimathvector.h#L82): ```cpp /** * @brief Method that fills a vector with a value * * @param v value of which the vector is filled * @return vector of type PIMathVectorT filled with "v" */ _CVector & fill(const Type & v) {PIMV_FOR(i, 0) c[i] = v; return *this;} ``` В блоке `@return` описание слишком расплывчато и не отражает того факта, что возвращаемый вектор- это тот же самый вектор, у которого был вызван метод `fill()`. Необходимо уточнить в документации логику возвращаемого значения для классов `PIMathMatrixT`, `PIMathMatrix`, `PIMathVectorT`, `PIMathVector`
Contributor

Нужно постараться избегать рекурсивных определений. Пример PIMathVector::length() и PIMathVector::size(). По описанию невозможно понять что делают методы.

Нужно постараться избегать рекурсивных определений. Пример [PIMathVector::length()](https://git.shs.tools/SHS/pip/src/commit/b7f035178f62ca6f3d7042cc39ebc9711904f6ac/libs/main/math/pimathvector.h#L665) и [PIMathVector::size()](https://git.shs.tools/SHS/pip/src/commit/b7f035178f62ca6f3d7042cc39ebc9711904f6ac/libs/main/math/pimathvector.h#L599). По описанию невозможно понять что делают методы.
Contributor

В тестах заменить магические константы чем то осмысленным.

В тестах заменить магические константы чем то осмысленным.
Contributor

Сравнение объектов класса PIMathVector<Type> с вещественными типами через оператор == производится не корректно, сравнивая элементы на прямую через ==. Вместо этого необходимо сравнивать элементы по правилам сравнения вещественных типов.

Сравнение объектов класса `PIMathVector<Type>` с вещественными типами через [оператор `==`](https://git.shs.tools/SHS/pip/src/commit/b7f035178f62ca6f3d7042cc39ebc9711904f6ac/libs/main/math/pimathvector.h#L797) производится не корректно, сравнивая элементы на прямую через `==`. Вместо этого необходимо сравнивать элементы по правилам сравнения вещественных типов.
zzuummaa approved these changes 2020-09-24 14:38:07 +03:00
@@ -71,1 +256,4 @@
_CVector & operator =(const _CVector & v) {memcpy(c, v.c, sizeof(Type) * Size); return *this;}
/**
* @brief Vector assignment to value "v"
Contributor

Нечего не понятно

Нечего не понятно
@@ -175,1 +625,4 @@
_CVector & fill(const Type & v) {PIMV_FOR(i, 0) c[i] = v; return *this;}
/**
* @brief Method that fills a vector with the adittion of vector value and "v"
Contributor

Ничего не понятно.

Ничего не понятно.
@@ -177,1 +638,4 @@
* @param v vector of type PIMathVectorT
* @return vector of type PIMathVectorT with values adittion of vector value and "v"
*/
_CVector & move(const _CVector & v) {PIMV_FOR(i, 0) c[i] += v[i]; return *this;}
Contributor

Ничего не понятно по документации.
В документации не отражен случай когда размерности векторов не равны.
В коде не учитен случай, когда размерности векторов не равны.

Ничего не понятно по документации. В документации не отражен случай когда размерности векторов не равны. В коде не учитен случай, когда размерности векторов не равны.
@@ -198,1 +778,4 @@
* @param v vector for the assigment
* @return vector equal to vector "v"
*/
_CVector & operator =(const _CVector & v) {c = v.c; return *this;}
Contributor

Ничего не понятно в документации.
Не описан случай несовпадения размерностей.
Нет проверки размерностей в коде

Ничего не понятно в документации. Не описан случай несовпадения размерностей. Нет проверки размерностей в коде
@@ -202,1 +809,4 @@
*
* @param v vector for the addition assigment
*/
void operator +=(const _CVector & v) {PIMV_FOR(i, 0) c[i] += v[i];}
Contributor

Ничего не понятно в документации.
В документации не описан случай несовпадения размерностей.
Код не обрабатывает случай несовпадения размерностей.

Аналогично:
pimathvector.h#L819
pimathvector.h#L833
pimathvector.h#L847
pimathvector.h#L862
pimathvector.h#L869

и т.д.

Ничего не понятно в документации. В документации не описан случай несовпадения размерностей. Код не обрабатывает случай несовпадения размерностей. Аналогично: [pimathvector.h#L819](https://git.shs.tools/SHS/pip/src/commit/b7f035178f62ca6f3d7042cc39ebc9711904f6ac/libs/main/math/pimathvector.h#L819) [pimathvector.h#L833](https://git.shs.tools/SHS/pip/src/commit/b7f035178f62ca6f3d7042cc39ebc9711904f6ac/libs/main/math/pimathvector.h#L833) [pimathvector.h#L847](https://git.shs.tools/SHS/pip/src/commit/b7f035178f62ca6f3d7042cc39ebc9711904f6ac/libs/main/math/pimathvector.h#L847) [pimathvector.h#L862](https://git.shs.tools/SHS/pip/src/commit/b7f035178f62ca6f3d7042cc39ebc9711904f6ac/libs/main/math/pimathvector.h#L862) [pimathvector.h#L869](https://git.shs.tools/SHS/pip/src/commit/b7f035178f62ca6f3d7042cc39ebc9711904f6ac/libs/main/math/pimathvector.h#L869) и т.д.
@@ -215,1 +906,4 @@
* @param v is vector for dot product
* @return resulting vector
*/
Type operator ^(const _CVector & v) const {Type tv(0); PIMV_FOR(i, 0) tv += c[i] * v[i]; return tv;}
Contributor

Документация не соответствует коду: вычисляется скалярное произведение а не его модуль.
Не описан случай несовпадения разменостей.
Код не учитывает случай несовпадения размерностей.

Документация не соответствует коду: вычисляется скалярное произведение а не его модуль. Не описан случай несовпадения разменостей. Код не учитывает случай несовпадения размерностей.
@@ -217,1 +915,4 @@
* @param lp1 is vector
* @return resulting value
*/
Type distToLine(const _CVector & lp0, const _CVector & lp1) {
Contributor

Документация не соответствует коду. Метод вычисляет расстояние от точки до прямой, задаваемой lp0 и lp1
Описать параметры и возвращаемое значение подробнее.

Документация не соответствует коду. Метод вычисляет расстояние от точки до прямой, задаваемой `lp0` и `lp1` Описать параметры и возвращаемое значение подробнее.
@@ -0,0 +4,4 @@
bool cmpVectorWithValue(PIMathVector<double> vector, double val, int num) {
bool b = true;
for(int i = 0; i < num; i++) {
if(vector[i] != val) {
Contributor

Не корректно сравнивать значения типа double оператором != (см. обсуждение).

Не корректно сравнивать значения типа `double` оператором `!=` ([см. обсуждение](https://ru.stackoverflow.com/questions/461464/%D0%9A%D0%B0%D0%BA-%D1%81%D1%80%D0%B0%D0%B2%D0%BD%D0%B8%D1%82%D1%8C-%D0%B2%D0%B5%D1%89%D0%B5%D1%81%D1%82%D0%B2%D0%B5%D0%BD%D0%BD%D1%8B%D0%B5-%D1%87%D0%B8%D1%81%D0%BB%D0%B0-%D0%B2-%D0%A1%D0%B8-%D0%BD%D0%B0-%D0%B1%D0%BE%D0%BB%D1%8C%D1%88%D0%B5-%D0%BC%D0%B5%D0%BD%D1%8C%D1%88%D0%B5)).
zzuummaa marked this conversation as resolved
@@ -0,0 +13,4 @@
TEST(PIMathVector_Test, size) {
auto vector = PIMathVector<double>(3u);
ASSERT_TRUE(vector.size() == 3u);
Contributor

Предпочтительнее использовать ASSERT_EQ

Предпочтительнее использовать ASSERT_EQ
@@ -0,0 +16,4 @@
ASSERT_TRUE(vector.size() == 3u);
}
TEST(PIMathVector_Test, resize) {
Contributor

Нет проверки на size() вектора перед проверкой значений.

Нет проверки на size() вектора перед проверкой значений.
@@ -0,0 +22,4 @@
ASSERT_TRUE(cmpVectorWithValue(vector, 5.0, vector.size()));
}
TEST(PIMathVector_Test, resized) {
Contributor

Нет проверки на size() вектора перед проверкой значений. Тест составлен не корректно.

Нет проверки на size() вектора перед проверкой значений. Тест составлен не корректно.
@@ -0,0 +60,4 @@
a[1] = vector[1];
a[2] = vector[2];
vector.swap(0u, 1u);
ASSERT_TRUE((a[0] == vector[1]) && (a[1] == vector[0]) && (a[2] == vector[2]));
Contributor

Не корректно сравнивать значения типа double оператором == (см. обсуждение). Заменить на EXPECT_DOUBLE_EQ.

Не корректно сравнивать значения типа `double` оператором `==` ([см. обсуждение](https://ru.stackoverflow.com/questions/461464/%D0%9A%D0%B0%D0%BA-%D1%81%D1%80%D0%B0%D0%B2%D0%BD%D0%B8%D1%82%D1%8C-%D0%B2%D0%B5%D1%89%D0%B5%D1%81%D1%82%D0%B2%D0%B5%D0%BD%D0%BD%D1%8B%D0%B5-%D1%87%D0%B8%D1%81%D0%BB%D0%B0-%D0%B2-%D0%A1%D0%B8-%D0%BD%D0%B0-%D0%B1%D0%BE%D0%BB%D1%8C%D1%88%D0%B5-%D0%BC%D0%B5%D0%BD%D1%8C%D1%88%D0%B5)). Заменить на `EXPECT_DOUBLE_EQ`.
zzuummaa marked this conversation as resolved
@@ -0,0 +63,4 @@
ASSERT_TRUE((a[0] == vector[1]) && (a[1] == vector[0]) && (a[2] == vector[2]));
}
TEST(PIMathVector_Test, lengthSqr) {
Contributor

Лучше заполнять вектор чем то не равным еденице.
Не корректно сравнивать значения типа double оператором ==. Заменить на EXPECT_DOUBLE_EQ.

Лучше заполнять вектор чем то не равным еденице. Не корректно сравнивать значения типа `double` оператором `==`. Заменить на EXPECT_DOUBLE_EQ.
@@ -0,0 +69,4 @@
ASSERT_EQ(3.0, vector.lengthSqr());
}
TEST(PIMathVector_Test, length) {
Contributor

Лучше заполнять вектор чем то не равным еденице.

Лучше заполнять вектор чем то не равным еденице.
@@ -0,0 +96,4 @@
vector[0] = 1.0;
vector[1] = 1.0;
vec[1] = 1.0;
ASSERT_DOUBLE_EQ(cos(0.78539816339744830961566084581988), vector.angleSin(vec));
Contributor

Вычисление косинуса вводит в заблуждение.

Вычисление косинуса вводит в заблуждение.
@@ -0,0 +117,4 @@
ASSERT_DOUBLE_EQ(45.0, vector.angleDeg(vec));
}
TEST(PIMathVector_Test, projection) {
Contributor

Посчитать проекцию в коде и сравнить

Посчитать проекцию в коде и сравнить
@@ -0,0 +133,4 @@
ASSERT_TRUE(cmpVectorWithValue(vector.normalize(), 5.0 / sqrt(75.0), 3u));
}
TEST(PIMathVector_Test, normalized) {
Contributor

Тест составлен не корректно.

Тест составлен не корректно.
@@ -0,0 +328,4 @@
PIMathVector<double> vector2(3u);
vector1[0] = 1.0;
vector2[1] = 1.0;
ASSERT_TRUE(((vector1 * vector2)[0] == 0.0) && ((vector1 * vector2)[1] == 0.0) && ((vector1 * vector2)[2] == 1.0));
Contributor

Привести к читаемой форме.
Не нужно умножать векторы 3 раза.

Привести к читаемой форме. Не нужно умножать векторы 3 раза.
@@ -0,0 +331,4 @@
ASSERT_TRUE(((vector1 * vector2)[0] == 0.0) && ((vector1 * vector2)[1] == 0.0) && ((vector1 * vector2)[2] == 1.0));
}
TEST(PIMathVector_Test, operator_DivisionVector) {
Contributor

Тест должен называться operator_DivisionValue.
Написать тест operator_DivisionVector.

Тест должен называться `operator_DivisionValue`. Написать тест `operator_DivisionVector`.
andrey requested review from zzuummaa 2020-09-24 14:39:45 +03:00
andrey removed review request for zzuummaa 2020-09-24 14:55:23 +03:00
maakshishov was assigned by andrey 2020-10-21 13:22:11 +03:00
Author
Owner

закрываем, будем делать ещё раз сначала

закрываем, будем делать ещё раз сначала
andrey closed this pull request 2020-10-27 15:46:44 +03:00

Pull request closed

Sign in to join this conversation.