3
\$\begingroup\$

I have a class Vector3d with 3 double components and want to store several of them in a Field. For reasons of efficiency when sending a Field over a network via MPI, I would like to allocate contiguous memory in the Field class and let the Vector3ds store their components in that chunk of memory. In addition, Vector3ds should also be able to live without a Field; in this case, they have to care about their own memory allocation.

#include <cstddef>
#include <iostream>
#include <vector>
class Field;
class Vector3d
{
 friend class Field;
 public:
 Vector3d() : components_(new double[3]) {}
 ~Vector3d() { delete[] components_; }
 Vector3d(const Vector3d &v) : components_(new double[3])
 {
 components_[0] = v.components_[0]; 
 components_[1] = v.components_[1]; 
 components_[2] = v.components_[2];
 }
 double & operator()(const std::size_t index) { return components_[index]; }
 private:
 Vector3d(double *ptr) : components_(ptr) {};
 double *components_;
};
class Field
{
 public: 
 Field(std::size_t size);
 ~Field();
 Vector3d & operator()(const std::size_t index) { return *(vectors_[index]); }
 private:
 std::vector<double> components_;
 std::vector<Vector3d *> vectors_;
};
Field::Field(std::size_t size) : components_(std::vector<double>(3 * size)), vectors_(std::vector<Vector3d *>(size, new Vector3d(0)))
{
 double *ptr = &components_[0];
 for (auto it = vectors_.begin(); it != vectors_.end(); ++it)
 {
 (*it)->components_ = ptr;
 ptr += 3;
 }
} 
Field::~Field()
{
 for (auto it = vectors_.begin(); it != vectors_.end(); ++it)
 (*it)->components_ = 0;
}
int main(int argc, char *argv[])
{
 Field field(2);
 Vector3d &v = field(1);
 v(2) = 5;
 Vector3d v2 = v;
 v2(2) = 2;
 std::cout << field(1)(2) << std::endl;
 return 0;
}

I would like to hear what you think about this piece of code. Can this be considered a good solution or what would you change?

Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Mar 10, 2015 at 19:24
\$\endgroup\$
5
  • \$\begingroup\$ Lets say you have a 2d vector std::vector<std::vector<double>> data which is basically what you have. If you wanted flatten it out you could use for( auto& e : data) flat_vector.insert(flat_vector.end(), e.begin(), e.end()) \$\endgroup\$ Commented Mar 10, 2015 at 19:35
  • 1
    \$\begingroup\$ Why not distinguish between vector3d_view and vector3d, where a _view is non-owning? As a bonus, a vector3d could store a double data[3];, and a std::vector<vector3d> would be contiguous memory already all by itself. Making your primitive types "not care about ownership" is a recipe for inefficiency and bugs in my experience. While casting a std::vector<vector3d> to a raw array of double is not permitted by the standard, it may work on your particular platform if vector3d contains nothing but the double data[3]; (is standard layout) \$\endgroup\$ Commented Mar 10, 2015 at 19:50
  • \$\begingroup\$ @DrewDormann: Ok, my example might be a bit too long but I just wanted to make myself clear. To boil it down: What I need is a vector of vectors but stored in contiguous memory. Since this is not possible using std::vector<std::vector<double>>, I thought illustrating my alternative with a piece of code would help people to understand my question better. \$\endgroup\$ Commented Mar 11, 2015 at 5:07
  • \$\begingroup\$ @NathanOliver: Thanks for that suggestion but I think your solution involves a copy operation each time I want to access the aggregate vector. Since I have to do this very often in my application, I don't think this would be the right way to go. \$\endgroup\$ Commented Mar 11, 2015 at 5:09
  • \$\begingroup\$ @Yakk: I also came up with that solution but then I would have to treat Vector3d_view and Vector3d differently which is not what I want. Inheriting from a common base class also seems to be a bit overkill. Your suggestion for casting std::vector<Vector3d> to a raw array of double is not an option because I'm planning to add some functionality to the Vector3d class which would destroy contiguous memory layout. \$\endgroup\$ Commented Mar 11, 2015 at 5:15

1 Answer 1

4
\$\begingroup\$

When it comes to multidimensional vectors I usually suggest two approaches.

1. vector/array (C++11 and later)

This only works, when the inner dimensions are fixed and only the outer is runtime dynamic. In the end it comes down to something like this:

std::vector<std::array<T, N>> field;

In your case a Vector3d would either derive from std::array<double, 3> or contain one.

2. simple vector

This approach simply creates a one dimensional vector with X * Y size. The element (x, y) can then be addressed with vec[y * X + x]. This works also with the older C++ standard. Although I think this solution might not work in your setup.

Bonus

I just want to highlight @Yakk's comment. Your Vector3d could look like this:

class Vector3d
{
public:
 // stuff
 // ...
private:
 double components_[3];
};

This way the memory would also be contigous.

answered Mar 12, 2015 at 13:27
\$\endgroup\$
9
  • \$\begingroup\$ Thanks for your suggestion. Regarding your first solution, you suggest that the Field class holds a std::vector<Vector3d>, which, due to the fact that the Vector3d only holds double components_[3] as a sole member could be casted to a raw double array? Can this really considered to be good style? If sometime I decide to add members this solution won't work anymore... Actually, your second solution is what I had already but it would be better to access the individual entities of a field individually. What about my proposed solution? Do you see any memory issues? \$\endgroup\$ Commented Mar 12, 2015 at 15:23
  • \$\begingroup\$ I had to say something more after inspecting your code, so I commented it on ideone. Also here is a version that I would call "clean", though that does not have to mean much. \$\endgroup\$ Commented Mar 12, 2015 at 21:25
  • \$\begingroup\$ Thanks for your comments. The idea was to create a Vector3d class which can also live on its own (and might have members in addition to components_). I know that new double[3] creates memory somewhere on the heap and that is what the default behavior should be. I don't see the memory leak (point me to it if I'm wrong!) because the Field class cares for memory allocation and deallocation. The Field class needs to store std::vector<Vector3d *> because I don't want the Vector3d's copy constructor to be invoked (which would allocate memory a second time). \$\endgroup\$ Commented Mar 13, 2015 at 7:18
  • \$\begingroup\$ In the initializer list of the Field constructor, the private constructor of Vector3d is called with a 0 pointer (only used by the Field class!). In the loop, the pointer is assigned a new location. Further, I don't see any issues using operator() instead of operator[], I think this is just personal taste. The same for const std::size_t, instead of std::size_t - it doesn't harm. I was just following the rule "put const wherever you can". I didn't implement the const version of the access operators because I didn't want to make the examples longer than necessary. \$\endgroup\$ Commented Mar 13, 2015 at 7:19
  • \$\begingroup\$ Regarding your solution, you essentially use a typedef to std::array<double, 3>, i.e., the a vector does not have any operators or the like. Of course, you could embed it in a class and let it have just a single member (its components) but as soon as you add just one other member, memory contiguity would be destroyed. I would be happy if you could elaborate this issue. \$\endgroup\$ Commented Mar 13, 2015 at 7:20

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.