-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Vector3 object #7
base: master
Are you sure you want to change the base?
Changes from all commits
855dfac
5e61b75
4374a7d
a3aa191
1efa81f
729cfac
f785285
2636be7
cff9b3b
8843f43
483c6b7
26d74ec
06e395d
e462e26
49a1546
ce3ea8c
d531cae
2cf69ca
e32671c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
/build | ||
/.vscode | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
{ | ||
"configurations": [ | ||
{ | ||
"name": "Linux", | ||
"includePath": [ | ||
"${workspaceFolder}/**" | ||
], | ||
"defines": [], | ||
"compilerPath": "/usr/bin/clang", | ||
"cStandard": "c11", | ||
"cppStandard": "c++17", | ||
"intelliSenseMode": "clang-x64" | ||
} | ||
], | ||
"version": 4 | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This file should probably not have made it into the repo, right? |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
#pragma once | ||
#include <iostream> | ||
#include "math.h" | ||
|
||
namespace ekumen | ||
{ | ||
|
||
class Vector3 | ||
{ | ||
public: | ||
explicit Vector3(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need to do this explicit. |
||
explicit Vector3(const double x, const double y, const double z); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add a copy constructo. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Apply the rule-of-five and create move operations: construct and assign. (this is why I wanted an allocated storage). |
||
Vector3(const std::initializer_list<double> &list); | ||
|
||
double x() const {return values[0];} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: This is fine, but you could instead return a |
||
double &x() {return values[0];} | ||
|
||
double y() const { return values[1]; } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here. |
||
double &y() {return values[1];} | ||
|
||
double z() const { return values[2]; } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And here. |
||
double &z() {return values[2];} | ||
|
||
|
||
|
||
double norm() const; | ||
double dot(const Vector3 &q) const; | ||
Vector3 cross(const Vector3 &q) const; | ||
//operator overload | ||
const double& operator[](const int index) const; | ||
double& operator[](const int index); | ||
|
||
|
||
Vector3 &operator+=(const Vector3 q); | ||
Vector3 &operator-=(const Vector3 q); | ||
|
||
|
||
friend Vector3 operator*(const double &cte, const Vector3 &p); | ||
friend Vector3 operator*(const Vector3 &p, const double &cte); | ||
friend Vector3 operator*(const Vector3 &p, const Vector3 &q); | ||
friend Vector3 operator/(const Vector3 &p, const Vector3 &q); | ||
|
||
friend bool operator==(const Vector3 &p, const Vector3 &q); | ||
friend bool operator==(const Vector3 &p, const std::initializer_list<double> &list); | ||
|
||
friend bool operator!=(const Vector3 &p, const Vector3 &q); | ||
friend bool operator!=(const Vector3 &p, const std::initializer_list<double> &list); | ||
|
||
friend std::ostream &operator<<(std::ostream &os, const Vector3 &p); | ||
|
||
|
||
static const Vector3 kUnitX; | ||
static const Vector3 kUnitY; | ||
static const Vector3 kUnitZ; | ||
static const Vector3 kZero; | ||
|
||
private: | ||
|
||
double *values = new double[3]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you don't release this memory somewhere, this memory will get leaked everytime you create one of these objects. |
||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These line breaks are redundant and will usually be removed by an auto-formatter. |
||
}; | ||
|
||
Vector3 operator+(Vector3 p, Vector3 q); | ||
Vector3 operator-(Vector3 p, Vector3 q); | ||
|
||
|
||
} // namespace ekumen |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
#pragma once | ||
namespace cppcourse { | ||
|
||
|
||
} // cppcourse |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,156 @@ | ||
#include "Vector3.h" | ||
|
||
namespace ekumen | ||
{ | ||
|
||
Vector3::Vector3(const double x, const double y, const double z) | ||
{ | ||
values[0] = x; | ||
values[1] = y; | ||
values[2] = z; | ||
} | ||
Vector3::Vector3() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Replace this constructor with a delegating constructor that uses the one above: https://www.learncpp.com/cpp-tutorial/8-6-overlapping-and-delegating-constructors/ |
||
{ | ||
values[0] = 0.; | ||
values[1] = 0.; | ||
values[2] = 0.; | ||
} | ||
Vector3::Vector3(const std::initializer_list<double> &list){ | ||
values[0] = *(list.begin() + 0); | ||
values[1] = *(list.begin() + 1); | ||
values[2] = *(list.begin() + 2); | ||
} | ||
double Vector3::norm() const | ||
{ | ||
return sqrt(dot(Vector3(x(), y(), z()))); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can be |
||
} | ||
|
||
double Vector3::dot(const Vector3 &q) const | ||
{ | ||
return ((values[0] * q.x()) + (values[1] * q.y()) + (values[2] * q.z())); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For readability's sake you can use |
||
} | ||
|
||
Vector3 Vector3::cross(const Vector3 &q) const | ||
{ | ||
Vector3 r; | ||
r.x() = values[1] * q.z() - values[2] * q.y(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can be |
||
r.y() = values[2] * q.x() - values[0] * q.z(); | ||
r.z() = values[0] * q.y() - values[1] * q.x(); | ||
return (r); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need for the parenthesis. |
||
} | ||
|
||
Vector3 &Vector3::operator+=(const Vector3 q){ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The argument can be |
||
x() += q.x(); | ||
y() += q.y(); | ||
z() += q.z(); | ||
return *this; | ||
} | ||
|
||
Vector3 &Vector3::operator-=(const Vector3 q){ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same, better add a |
||
x() -= q.x(); | ||
y() -= q.y(); | ||
z() -= q.z(); | ||
return *this; | ||
} | ||
|
||
Vector3 operator+(Vector3 p, Vector3 q) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Arguments should be |
||
{ | ||
Vector3 r; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can better use your (x, y, z) constructor, or better yet, your copy constructor. |
||
r.x() = p.x(); | ||
r.y() = p.y(); | ||
r.z() = p.z(); | ||
return (r+=q); | ||
} | ||
|
||
Vector3 operator-(Vector3 p, Vector3 q) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Arguments should be |
||
{ | ||
Vector3 r; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use your copy constructor. |
||
r.x() = p.x(); | ||
r.y() = p.y(); | ||
r.z() = p.z(); | ||
return (r-=q); | ||
} | ||
|
||
Vector3 operator*(const double &cte, const Vector3 &p) | ||
{ | ||
Vector3 res(cte * p.x(), cte * p.y(), cte * p.z()); | ||
return res; | ||
} | ||
|
||
Vector3 operator*(const Vector3 &p, const double &cte) | ||
{ | ||
Vector3 res(cte * p.x(), cte * p.y(), cte * p.z()); | ||
return res; | ||
} | ||
|
||
Vector3 operator*(const Vector3 &p, const Vector3 &q) | ||
{ | ||
Vector3 res(p.x() * q.x(), p.y() * q.y(), p.z() * q.z()); | ||
return res; | ||
} | ||
|
||
Vector3 operator/(const Vector3 &p, const Vector3 &q) | ||
{ | ||
Vector3 res(p.x() / q.x(), p.y() / q.y(), p.z() / q.z()); | ||
return res; | ||
} | ||
|
||
bool operator==(const Vector3 &p, const Vector3 &q) | ||
{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use the clang-format formatter, and if you're using vscode, you can add a plugin to use it from the interface to autoformat everything. |
||
bool res = (p.x() == q.x()) && (p.y() == q.y()) && (p.z() == q.z()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can sue |
||
return res; | ||
} | ||
|
||
bool operator==(const Vector3 &p, const std::initializer_list<double> &list){ | ||
bool res = (p.x() == *(list.begin() + 0)) && (p.y() == *(list.begin() + 1)) && (p.z() == *(list.begin() + 2)); | ||
return res; | ||
} | ||
|
||
bool operator!=(const Vector3 &p, const Vector3 &q) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Better leave line in between. |
||
{ | ||
|
||
return (!(p == q)); | ||
} | ||
|
||
bool operator!=(const Vector3 &p, const std::initializer_list<double> &list){ | ||
Vector3 q(*(list.begin() + 0),*(list.begin() + 1),*(list.begin() + 2)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To do this you need to call |
||
return (!(p == q)); | ||
|
||
} | ||
|
||
std::ostream &operator<<(std::ostream &os, const Vector3 &p) | ||
{ | ||
os << "(x: " << p.x() << ", y: " << p.y() << ", z: " << p.z() << ")"; | ||
return os; | ||
} | ||
|
||
const double &Vector3::operator[](const int index) const | ||
{ | ||
try | ||
{ | ||
return values[index]; | ||
} | ||
catch (const std::exception &ex) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add a test for this case in the test file. |
||
{ | ||
std::cerr << "Error occurred: " << ex.what() << std::endl; | ||
} | ||
} | ||
|
||
double &Vector3::operator[](const int index) | ||
{ | ||
try | ||
{ | ||
return values[index]; | ||
} | ||
catch (const std::exception &ex) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please also add a test for this case. |
||
{ | ||
std::cerr << "Error occurred: " << ex.what() << std::endl; | ||
} | ||
} | ||
|
||
const Vector3 Vector3::kUnitX = Vector3(1., 0., 0.); | ||
const Vector3 Vector3::kUnitY = Vector3(0., 1., 0.); | ||
const Vector3 Vector3::kUnitZ = Vector3(0., 0., 1.); | ||
const Vector3 Vector3::kZero = Vector3(0., 0., 0.); | ||
|
||
} // namespace ekumen |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ include_directories( | |
|
||
# Test sources. | ||
set (GTEST_SOURCES | ||
isometry_TEST.cc | ||
foo_TEST.cc | ||
) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,26 +3,27 @@ | |
// needed to implement an isometry. | ||
|
||
// Consider including other header files if needed. | ||
#include "isometry.h" | ||
|
||
//#include "isometry.h" | ||
#include "Vector3.h" | ||
#include <cmath> | ||
#include <sstream> | ||
#include <string> | ||
#include <initializer_list> | ||
|
||
#include "gtest/gtest.h" | ||
|
||
namespace ekumen { | ||
namespace math { | ||
namespace cppcourse { | ||
namespace test { | ||
namespace { | ||
|
||
GTEST_TEST(Vector3Test, Vector3Operations) { | ||
const double kTolerance{1e-12}; | ||
const Vector3 p{1., 2., 3.}; | ||
const Vector3 q{4., 5., 6.}; | ||
|
||
EXPECT_EQ(p + q, {5., 7., 9.}); | ||
EXPECT_EQ(p - q, {-3., -3., -3.}); | ||
EXPECT_EQ(p + q, std::initializer_list<double>({5., 7., 9.})); | ||
EXPECT_EQ(p - q, std::initializer_list<double>({-3., -3., -3.})); | ||
EXPECT_EQ(p * 2., Vector3(2., 4., 6)); | ||
EXPECT_EQ(2 * q, Vector3(8., 10., 12.)); | ||
EXPECT_EQ(p * q, Vector3(4., 10., 18.)); | ||
|
@@ -40,8 +41,8 @@ GTEST_TEST(Vector3Test, Vector3Operations) { | |
EXPECT_EQ(ss.str(), "(x: 1, y: 2, z: 3)"); | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need to add this line. |
||
EXPECT_TRUE(Vector3::kUnitX == Vector3(1., 0., 0)); | ||
EXPECT_TRUE(Vector3::kUnitX != {1., 1., 0}); | ||
EXPECT_TRUE(Vector3::kUnitY == {0., 1., 0}); | ||
EXPECT_TRUE(Vector3::kUnitX != std::initializer_list<double>({1., 1., 0})); | ||
EXPECT_TRUE(Vector3::kUnitY == std::initializer_list<double>({0., 1., 0})); | ||
EXPECT_TRUE(Vector3::kUnitZ == Vector3::kUnitX.cross(Vector3::kUnitY)); | ||
EXPECT_NEAR(Vector3::kUnitX.dot(Vector3::kUnitZ), 0., kTolerance); | ||
|
||
|
@@ -53,6 +54,7 @@ GTEST_TEST(Vector3Test, Vector3Operations) { | |
EXPECT_EQ(t, p); | ||
} | ||
|
||
/* | ||
GTEST_TEST(Matrix3Test, Matrix3Operations) { | ||
const double kTolerance{1e-12}; | ||
Matrix3 m1{{1., 2., 3.}, {4., 5., 6.}, {7., 8., 9.}}; | ||
|
@@ -96,7 +98,9 @@ GTEST_TEST(Matrix3Test, Matrix3Operations) { | |
ASSERT_TRUE(found); | ||
} | ||
} | ||
*/ | ||
|
||
/* | ||
GTEST_TEST(IsometryTest, IsometryOperations) { | ||
const double kTolerance{1e-12}; | ||
const Isometry t1 = Isometry::FromTranslation({1., 2., 3.}); | ||
|
@@ -129,9 +133,10 @@ GTEST_TEST(IsometryTest, IsometryOperations) { | |
ss << t5; | ||
EXPECT_EQ(ss.str(), "[T: (x: 0, y: 0, z: 0), R:[[0.923879533, -0.382683432, 0], [0.382683432, 0.923879533, 0], [0, 0, 1]]]"); | ||
} | ||
|
||
} // namespace | ||
*/ | ||
} | ||
} // namespace test | ||
} // namespace cppcourse | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably we don't need another namespace layer, but I'll leave it up to you. |
||
} // namespace math | ||
} // namespace ekumen | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm ok with this, but keep in mind that usually is not a good idea to update the .gitignore file in the clients repo to ignore files in your own personal computer setup.