0

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++.)

vitrack
  • 65
  • 5
  • 4
    Your `foo` class has incorrect copy semantics, as it does not follow the [rule of 3](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three), and go to the **Managing resources** section at that link (does the code look familiar?). -- *is there a better way to have a class with a variable length array as a parameter?* -- Use [std::vector](https://en.cppreference.com/w/cpp/container/vector) – PaulMcKenzie Oct 30 '22 at 12:39
  • Note : int *bar; is totally unecessary. It will only complicate things so avoid it. – Pepijn Kramer Oct 30 '22 at 12:41
  • 1
    *Please keep in mind when answering that I'm very new to c++* -- But you decided to get into the weeds of having to write a user-defined copy constructor and assignment operator. That is not what someone very new to C++ should be attempting to do, until you learn exactly how to handle dynamically allocated memory. – PaulMcKenzie Oct 30 '22 at 12:41
  • 1
    Does this answer your question? [What is The Rule of Three?](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) – Richard Critten Oct 30 '22 at 12:41
  • 1
    If you are new to C++, you should be using `std::vector`, and not be using the advanced memory management features of `new[]` and `delete[]` (or `new`, `delete`, `malloc`, or `free`). – Eljay Oct 30 '22 at 12:44
  • The issue with the link that you mentioned in your post is that it does not emphasize enough to use `std::vector`. Instead it seemed to have steered you down the wrong track into attempting to use `new[]` and `delete[]`, and all it did was open up another can of worms. I suggest you use good C++ books to learn from before trying to learn C++ by poking around looking at questions to give you ideas. Nothing wrong with doing this, *if* you have the experience to know what you're looking for. – PaulMcKenzie Oct 30 '22 at 12:46
  • Terminology: you cannot override variables; you override virtual functions. – Pete Becker Oct 30 '22 at 15:20
  • Also, don't use `l` as a name. It's often too hard to tell `l` and `1` apart. – Pete Becker Oct 30 '22 at 15:21

2 Answers2

1

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;
}
Lance
  • 169
  • 8
0

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)

vitrack
  • 65
  • 5