1

I have a class that should, as its input data, either use a reference to external data (without copying), or create the data itself based on other input. I prefer using references (to avoid dereferencing, since the data are matrices) and ended up with the following structure (simplified):

#include <iostream> #include <vector> using namespace std; using VectI = vector<int>; class A { VectI x0; VectI const & x = x0; public: A(VectI const & extX) : x(extX) {} // referencing existing external data A(int i) { x0.push_back(i); x0.push_back(i*i); } // create from other data void show_x() { cout << "x ="; for (auto&& i : x) cout << " " << i; cout << endl; } }; int main() { VectI myX = {1, 2, 3}; A a(myX); a.show_x(); // a references myX A b(2); b.show_x(); // b creates its own data based on i=2 return 0; }

The example works:

x = 1 2 3 x = 2 4

but are there any potential problems with this approach? In particular, is the fact that I am changing x0 that is referenced by the const vector x 'legal' C++, or is it something that other compilers might complain about?

Also, can I be sure that the first constructor avoids copying data?

Improve this question
4
  • 1
    There is no const vector in the code. x is a reference. Commented Jan 12, 2016 at 13:20
  • This may be a good question for Code Review, so long as: (A) the code works, and (B) it's not hypothetical or incomplete in any way. Please read the on-topic guide before posting, if you choose to go to Code Review. If you have any questions or concerns, join us at our CR Help Desk. Commented Jan 12, 2016 at 13:22
  • The vector is only considered const when accessed through the reference - the vector itself isn't const - so there are no problems with that, but note that the following will compile but has undefined behaviour: A a(VectI(3)); a.show_x();. Commented Jan 12, 2016 at 13:23
  • 5
    @πάντα ῥεῖ Thanks for the suggestion. I was not actually aware of Code Review .. but from their on-topic guide, it looks like they do not like simplified/minimal examples. Anyway, I got a nice answer from Bathsheba. Commented Jan 12, 2016 at 14:04

1 Answer 1

Reset to default
1

This is fine in the C++11 standard but your code is very brittle.

In particular the compiler generated copy and move constructors and the assignment operator will not work correctly, so you'll have to build your own.

You might also encounter dangling references: remember that the object lifetime extension due to a const reference is not transitive. The behaviour on using A(VectI const & extX) with an anonymous temporary is undefined.

Using a pointer to VectI - perhaps even a std::unique_ptr to a VectI along with a concept of ownership might be a safer way to go.

Improve this answer
Sign up to request clarification or add additional context in comments.

5 Comments

Thanks for info. As I have written above, I want to use references since it makes is much 'nicer' to access matrix elements (compared to accessing them from a pointer to the matrix). I do not really need copy or move constructor, or an assignment in my application, but thanks for the warning.
You can overload () for that nicety.
Really? Let's say I use shared_ptr<VectI> x. What would I be overloading to be able to write x(i)? Can I overload operator of pointer to something? And would it still work for boost/ublas matrices, which already use () for element access?
Yup. I don't get your aversion to overloading () for A. Also, I'd advise against breaking things like the constructors and assignment operators. You may (your your successor) may introduce them later.
I am sorry, but how would overloading () for A help with me having to write (*x)[i] instead of x[i], in all methods of A? (As I have said, the posted code is very simplified, I have several such data objects (matrices) inside my actual class and more methods, many of which work with the input matrices.)

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.