2

I don't understand raw pointers quite well and unfortunately I'm not allowed to use <vector>s, so I can't figure out how to write a parameterized constructor of a class that has an array of another class objects as its property.

class A{
    ...
};

class B{
    int size;
    A *arr;
    ...
    B(int, const A *); // am I declaring this right?
}

...

B::B(int size_, const A *arr_) { // this is the constructor I'm trying to write
    size = size_;
    if (arr != nullptr) delete[] arr;
    arr = new A[size];
    memcpy(arr, arr_, sizeof(A) * size);
}

How do I pass this argument without messing up the memory? The code above doesn't work properly, so I'd like to hear some tips. I didn't manage to google the solution although it seems like my question is already answered, I apologize in that case.

I'm not allowed to use any safe std:: stuff. I need to figure out how to make it work using manual memory allocation from C. Oops, I mean from C++, thanks for pointing this out.

So here's the constructor that is working for me so far: 

B::B(int size_, const A *arr_) {
    size = size_;
    arr = new A[size_];
    for (int i = 0; i < size; i++) arr[i] = arr_[i];
}

Thanks for everyone's time!

yyvv
  • 23
  • 3
  • You'll need to provide a full test case. See here: http://stackoverflow.com/help/mcve There isn't enough information in the code you provided to tell you what is wrong. – xaxxon Oct 11 '16 at 23:09
  • 1
    can you use `std:array`? – Bryan Chen Oct 11 '16 at 23:09
  • 1
    `if (arr != nullptr) delete[] arr;` is unlikely what you want in your constructor. Well, it is just wrong, actually. And using `memcpy` is just wrong unless you know that `A` is trivially copyable. – jxh Oct 11 '16 at 23:10
  • _"I need to figure out how to make it work using manual memory allocation from C."_ I'm very sorry to hear that. Note that neither `new` nor `delete` exist in C. – Lightness Races in Orbit Oct 11 '16 at 23:34
  • It's a good idea to show variable names in the class definition; so that someone reading the header file has an idea of what the parameters mean – M.M Oct 11 '16 at 23:39

2 Answers2

3

In your constructor:

if (arr != nullptr) delete[] arr;

arr is a class member that's not initialized at this point. It is a garbage pointer, likely to be something other than a nullptr, and this is going to attempt to delete the garbage pointer. Obviously, this is not going to go very far.

Simply remove this. Your constructor sets size and arr, and that's all that it needs to do. There are no existing values of size and arr to worry about: you're constructing a new object.

memcpy(arr, arr_, sizeof(A) * size);

This will only work if A is a POD. Instead, use std::copy(), which will do the right thing in all cases. If you can't use std::copy() in addition not being allowed to use std::vector, then write a for-loop to copy this data, manually.

Community
  • 1
  • 1
Sam Varshavchik
  • 114,536
  • 5
  • 94
  • 148
  • 1
    Worth noting that this still isn't great, since it default constructs A, and then copy-assigns, but it's good enough for the level the OP appears to be at. – Mooing Duck Oct 12 '16 at 00:50
0

There is no need to check if arr is NULL in the constructor because nothing has been assigned to arr yet.

memcpy is not used for copying classes because classes typically have member functions which are stored as function pointers. your memcpy statement should not be used as it will overwrite function pointers that are created in arr when the new A[size] command is executed and will not let class B execute properly.

Using a for loop to iterate through arr_ and copying only the data members of A from arr_ to arr is the way to go.

For example in the for loop you can do, arr[i] = arr_[i], where i is the index in the for loop. If class A does not have an overloaded assignment operator, a shallow copy of the member variables will happen. If a shallow copy is not acceptable, then an overloaded assignment operator should be created in class A if it does not exist.

Vivek
  • 320
  • 1
  • 5