Skip to content
This repository has been archived by the owner on Nov 17, 2021. It is now read-only.

Commit

Permalink
Remove direct access to internal data
Browse files Browse the repository at this point in the history
  • Loading branch information
jkflying authored and dagar committed Sep 17, 2019
1 parent 18218c8 commit 51d2f9f
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 28 deletions.
13 changes: 2 additions & 11 deletions matrix/Matrix.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,10 @@ template<typename Type, size_t M, size_t N>
class Matrix
{

public:

Type _data[M][N] {};

public:

// Constructors
Matrix() = default;

Expand Down Expand Up @@ -71,15 +71,6 @@ class Matrix
* Accessors/ Assignment etc.
*/

Type *data()
{
return _data[0];
}

const Type *data() const
{
return _data[0];
}

inline Type operator()(size_t i, size_t j) const
{
Expand Down
4 changes: 2 additions & 2 deletions test/attitude.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,8 @@ int main()
A.renormalize();
float err = 0.0f;

for (auto & row : A._data) {
Vector3f rvec(row);
for (size_t r = 0; r < 3; r++) {
Vector3f rvec(matrix::Matrix<float,1,3>(A.row(r)).transpose());
err += fabs(1.0f - rvec.length());
}
TEST(err < eps);
Expand Down
18 changes: 12 additions & 6 deletions test/matrixAssignment.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,18 @@ int main()
float data[9] = {1, 2, 3, 4, 5, 6, 7, 8, 9};
Matrix3f m2(data);

for(int i=0; i<9; i++) {
TEST(fabs(data[i] - m2.data()[i]) < FLT_EPSILON);
for(size_t i=0; i<3; i++) {
for (size_t j = 0; j < 3; j++) {
TEST(fabs(data[i*3 + j] - m2(i,j)) < FLT_EPSILON);
}
}

Matrix3f m_nan;
m_nan.setNaN();
for(int i=0; i<9; i++) {
TEST(isnan(m_nan.data()[i]));
for(size_t i=0; i<3; i++) {
for (size_t j = 0; j < 3; j++) {
TEST(isnan(m_nan(i,j)));
}
}
TEST(m_nan.isAllNan());

Expand All @@ -38,8 +42,10 @@ int main()
{7, 8, 9}
};
m2 = Matrix3f(data2d);
for(int i=0; i<9; i++) {
TEST(fabs(data[i] - m2.data()[i]) < FLT_EPSILON);
for(size_t i=0; i<3; i++) {
for (size_t j = 0; j < 3; j++) {
TEST(fabs(data[i*3 + j] - m2(i,j)) < FLT_EPSILON);
}
}
TEST(!m2.isAllNan());

Expand Down
7 changes: 0 additions & 7 deletions test/matrixMult.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,6 @@ int main()
float data[9] = {1, 0, 0, 0, 1, 0, 1, 0, 1};
Matrix3f A(data);

const Matrix3f Const(data);
const float * raw_data = Const.data();
const float eps = 1e-4f;
for (int i=0; i<9; i++) {
TEST(fabs(raw_data[i] - data[i]) < eps);
}

float data_check[9] = {1, 0, 0, 0, 1, 0, -1, 0, 1};
Matrix3f A_I(data_check);
Matrix3f I;
Expand Down
4 changes: 2 additions & 2 deletions test/upperRightTriangle.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ int main()

SquareMatrix<float, 3> A(data);

for(int i=0; i<6; i++) {
TEST(fabs(urt[i] - A.upper_right_triangle().data()[i]) < FLT_EPSILON);
for(size_t i=0; i<6; i++) {
TEST(fabs(urt[i] - A.upper_right_triangle()(i)) < FLT_EPSILON);
}

return 0;
Expand Down

2 comments on commit 51d2f9f

@jgoppert
Copy link
Member

Choose a reason for hiding this comment

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

@jkflying @dagar I used the direct data access for some generated code. My code expects a float pointer to the first data element. Can we add it back with a warning for use, or rename it _data() ?

@jkflying
Copy link
Collaborator Author

@jkflying jkflying commented on 51d2f9f Nov 18, 2019

Choose a reason for hiding this comment

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

You can still get a float pointer to the first element, just do it directly, ie. &mat(0,0), at least with the non-const operator. If we change the const operator to return a const Type & then it will work for that too.

Please sign in to comment.