Following the first answer to this question, I ran into an issue with overriding variables. This is the code I wrote to find the root of the problem:
#include <iostream>
using namespace std;
class foo
{
private:
int *bar;
int l;
public:
foo(int bar);
~foo();
void print_bar() {
cout << "bar = {"; for (int i = 0; i < l; i ) {cout << bar[i] << ", ";} cout << "}" << endl;
}
};
foo::foo(int l) {
this->l = l;
cout << "Creating with length " << l << endl;
bar = new int[l];
cout << "{"; for (int i = 0; i < l; i ) {cout << bar[i] << ", ";} cout << "}" << endl;
for(size_t i = 0; i < l; i) {bar[i] = 0;}
cout << "Created with bar = {"; for (int i = 0; i < l; i ) {cout << bar[i] << ", ";} cout << "}" << endl;
}
foo::~foo() {
cout << "Deleted with bar = {"; for (int i = 0; i < l; i ) {cout << bar[i] << ", ";} cout << "}" << endl;
delete[] bar;
}
int main() {
foo a = foo(1);
a = foo(2);
a.print_bar();
return 0;
}
This outputs:
Creating with length 1
{0, }
Created with bar = {0, }
Creating with length 2
{0, 0, }
Created with bar = {0, 0, }
Deleted with bar = {0, 0, }
bar = {1431655787, 5, }
Deleted with bar = {1431655787, 5, }
Instead of what I expected:
Creating with length 1
{0, }
Created with bar = {0, }
Creating with length 2
{0, 0, }
Created with bar = {0, 0, }
Deleted with bar = {0, }
bar = {0, 0, }
Deleted with bar = {0, 0, }
How can I avoid this, or is there a better way to have a class with a variable length array as a parameter?
(Please keep in mind when answering that I'm very new to c .)
CodePudding user response:
in the line, a=foo(2)
, the default assignment was called, but the service it provided is not right in your case(it just copy memory byte to byte). You need to supply a customised copy of assignment operator, in which you need to destruct *this
first.
class foo{
...
public:
foo& operator= (const foo& f)
{
this->~foo(); // call the destructor explicitly !
bar = nullptr; l= 0; // for exception safety.
new (this)(f); // call the copy constructor to construct
// a foo object on this which can be regarded as
// raw memory (it's still a valid foo object)
// but treating it as raw memory has no harm
// as all resources (here memory) has been released.
// btw, this is placement new,
// #include <new>
// before you can use it.
return *this;
}
// better yet in your case, a move assignment will be more efficient
foo& operator = (foo&& f)
{
std::swap(*this, f); // this lazy way will do for your purpose
// experiment and see what actually happened
// and figure out why it's like that
return *this;
}
My bad. You don't have a copy constructor defined yet. You can define one like this.
foo(const foo& f):foo(f.l)
{
for(int i=0; i<l; i)
bar[i] = f.bar[i];
}
And in the main
function
int main() {
foo a = foo(1);
foo b = foo(3);
a = foo(2); // this will call the move assignment operator
a = b; // this will call the copy assignment operator
a = std::move(b); // this will call the move assignment operator
// make sure you experiment with all likelyhood
// and understand the logic behind.
// don't trust me or anybody without actual trying
// and playing
a.print_bar();
return 0;
}
CodePudding user response:
Thanks to PaulMcKenzie and Eljay in the comments, I switched to std::vector to use as the variable length array. Here's the updated (and working as expected) code for anyone confused:
#include <iostream>
#include <vector>
using namespace std;
class foo
{
private:
vector<int> bar;
int l;
public:
foo(int bar);
~foo();
void print_bar() {
cout << "bar = {"; for (int i = 0; i < l; i ) {cout << bar[i] << ", ";} cout << "}" << endl;
}
};
foo::foo(int l) {
this->l = l;
cout << "Creating with length " << l << endl;
for(size_t i = 0; i < l; i) {bar.push_back(0);}
cout << "Created with bar = {"; for (int i = 0; i < l; i ) {cout << bar[i] << ", ";} cout << "}" << endl;
}
foo::~foo() {
cout << "Deleted with bar = {"; for (int i = 0; i < l; i ) {cout << bar[i] << ", ";} cout << "}" << endl;
}
int main() {
foo a = foo(1);
a = foo(2);
a.print_bar();
return 0;
}
(foo::~foo()
doesn't have to be defined, I just left it there for consistency)