Thắc mắc về operator+ và operator=

ko có copy constructor. Viết copy ctor cho Array1D đi đã.

Mình thêm copy constructor vào rồi, nhưng nó liên quan gì tới operator + hả bạn?
Ok mình hiểu rồi, do z = null chưa khởi tạo nên gán z = x + y bị lỗi…

Array1D operator+(Array1D x)

z = x + y;

truyền x kiểu này thì x là 1 bản copy của y trong phép cộng x + y kia. Ko có copy ctor thì con trỏ a trong bản copy này và con trỏ a trong y đều trỏ tới chung 1 mảng.

Khi thực hiện xong phép cộng thì bản copy kia bị xóa => mảng mà con trỏ a trong y cũng bị xóa => con trỏ a trong y trỏ tới mảng đã giải phóng rồi => khi gọi y.~Array1D(); thì ko thể xóa mảng này 1 lần nữa: _BLOCK_TYPE_IS_VALID(pHead->nBlockUse) tức là kiểm tra block/mảng đó có đang đang sử dụng / valid ko, mà trường hợp này đã được giải phóng rồi => invalid => lỗi

ko bao giờ gọi destructor trực tiếp, mà để chương trình tự động gọi. Trong trường hợp này dù ko gọi dtor trực tiếp thì dtor của y cũng đc gọi sau khi kết thúc chương trình, cũng báo lỗi, do ko có copy ctor đàng hoàng

ngoài ra:

• tất cả biến truyền vào hàm trừ phi có kiểu đơn giản như int, float, char, double, … thì mới truyền bản copy, còn lại nên truyền tham chiếu const& hết, ví dụ kiểu T, biến x thì truyền const T& x. Nếu có chỉnh sửa biến truyền vào thì bỏ const đi: T& x. Trừ vài trường hợp đặc biệt…
• operator= giá trị trả về nên là vế trái của phép gán: vd ở đây nên là Array1D& operator=(const Array1D& x); để có thể gán liên tiếp a = b = c. Hoặc nếu có hàm swap thì có thể xài Array1D& operator=(Array1D x); (trường hợp đặc biệt ko truyền const& đây…) trong thân hàm chỉ việc swap *this và x là xong.

Bạn chưa cấp phát vùng nhớ cho thuộc tính int* a. Bạn chỉ cần viết lại giống copy constructor là được.

Array1D& operator=(const Array1D& x) {
	this->n = x.n;
	this->a = new int[x.n];
	for (int i = 0; i < this->n; i++) {
		this->a[i] = x.a[i];
	}
        return *this;
}

Nếu bạn để ý thì mình đã sửa lại khai báo của operator= (thêm kiểu trả về là Array1D& thay vì void). Làm như vậy thì ta sẽ sử dụng operator= một cách “tự nhiên” nhất, ví dụ:

z = x = y;

Khai báo ban đầu của bạn không thể chạy được như trên vì bạn không có giá trị trả về cho operator=

Thêm nữa, ở cuối chương trình, bạn không cần phải gọi destructor của các đối tượng x, y, z vì nó sẽ được tự động gọi khi ra khỏi scope, tức là cặp ngoặc {} (bạn có thể kiểm tra bằng cách in ra thông báo trong destructor). Chỉ khi nào bạn dùng con trỏ để tạo đối tượng thì lúc đó bạn mới cần gọi, ví dụ

Array1D* a = new Array1D(); //tạo
delete a; // hủy (gọi destructor)

Ủa nếu vậy thì trong operator= mình phải return cái gì? Mình return this không được.

Xin lỗi, mình viết thiếu, *return this bạn nhé!

operator= rắc rối hơn 1 tí.

this->a = new int[x.n];
sau dòng này thì mảng mà a trỏ tới trước đó chưa được giải phóng => tràn bộ nhớ (leak memory)

=> phải delete [] this->a; trước khi cấp phát mảng mới.

nhưng delete như vậy lại gặp phải vấn đề: nếu người sử dụng viết
x = x;
gọi là “self-assignment”, dịch nôm na là tự gán cho bản thân, thì mảng mà x.a trỏ tới bị xóa ngay trước khi copy mảng bên vế phải vào mảng this->a mới, nên dòng
this->a[i] = x.a[i];
là vô nghĩa vì x.a[i] đã bị xóa (delete [] this->a; ở đây đồng nghĩa với delete x.a; vì object bên vế phải cũng chính là object bên vế trái).

=> phải kiểm tra self-assignment trước khi gán:
if (this == &x) return *this;
nếu con trỏ this cũng chính là con trỏ tới x thì khỏe re khỏi cần gán gì cả, return *this luôn.

Array1D& operator=(const Array1D& x)
{
    if (this == &rhs) return *this; //kiểm tra self-assignment
    this->n = x.n;
    delete [] this->a; //xóa mảng cũ
    this->a = new int[x.n];
    for (int i = 0; i < this->n; i++) 
        this->a[i] = x.a[i];
    return *this;
}

.
.
.
.
.
nhưng lại gặp phải vấn đề nữa =) Vấn đề ở đây ko phải là lỗi tràn bộ nhớ mà là về tốc độ (performance). Mỗi lần gán lại phải kiểm tra self-assignment khá là tốn (1 lệnh if, 1 lệnh lấy địa chỉ vế phải) mà 99% là vô ích, vì chả mấy khi dùng self-assignment. Ngoài ra còn vấn đề nữa là cấp phát bộ nhớ trong phép gán. Nếu chuyển cái cấp phát này qua phương thức khởi tạo (ctor) thì đỡ hơn. Theo logic thì ctor mới đc quyền cấp phát bộ nhớ, mấy phương thức khác cấp phát làm gì? Thêm cái nữa là phải giải phóng mảng cũ. Nếu chuyển cái này qua destructor (dtor) thì khỏe hơn.

=> copy-and-swap:

Array1D& operator=(Array1D x)
{
   //hoán đổi (swap) n
   int tempN = this->n;
   this->n = x.n;
   x.n = tempN;
   //hoán đổi mảng mà con trỏ a trỏ tới
   int* tempA = this->a;
   this->a = x.a;
   x.a = tempA;
}

• hoán đổi *this với bản copy của vế phải phép gán. Vì là bản copy nên hoàn toàn ko sợ self-assignment.
• ko có lệnh if.
• copy object mới cồng kềnh? Thật ra phép gán cũng tương đương với tạo 1 object mới rồi. Copy-and-swap cũng chỉ tạo có 1 object mới là bản copy của vế phải.
• ko phải xài new, ko phải bận tâm cấp phát mảng mới, khỏi mất công viết vòng for copy từng phần tử: copy ctor khi tạo 1 bản copy vế phải đã làm việc đó cho bạn.
• ko phải xài delete, ko phải bận tâm quét dọn mảng cũ: khi kết thúc phép gán thì x sẽ tự động hủy và giải phóng mảng cũ đã được hoán đổi vào x.
• cấp phát và giải phóng được chuyển về ctor và dtor hết.

Array1D& operator=(Array1D x)
{
//hoán đổi (swap) n
int tempN = this->n;
this->n = x.n;
x.n = tempN;
//hoán đổi mảng mà con trỏ a trỏ tới
int* tempA = this->a;
this->a = x.a;
x.a = tempA;
}

Giờ mới thấy cách xây dựng toán tử mới này, rất là hay nhưng em nghĩ :

• ít khi ta gán nó cho chính nó lắm, nên tránh hết sức có thể và khi đã dùng toán tử gán thì mình đã xác định là phải delete cái mảng cũ đi. Nên như bạn nói “99% là vô ích” khi kiểm tra self-assignment. Vậy chi bằng không kiểm tra sefl assignment nữa thì performance chắc nó hơn dùng copy và swap. tại copy thì nó cũng copy object , dùng thêm 2 biến tạm nữa.
  Dù sao e rất cám ơn bác vì kiến thức mới

2 biến tạm ko là bao nhiêu hết đó. Nếu Array1D có 10 ngàn phần tử thì nội việc copy là hết 10 ngàn bước rồi, có thêm 2 bước gán biến tạm nữa chẳng là bao nhiêu. Cũng có thể nói rằng như vậy 1 if so với 10 ngàn phép gán thì cũng chẳng nhằm nhò gì, cũng có lý, nhưng phải viết lại code y hệt như copy ctor trong operator= mà còn thêm với phần code giải phóng trong dtor nữa (nếu đây ko phải là array mà là linked list thì sao?), vậy là bị trùng code rồi.

hơn nữa với copy-and-swap thì có thể tách ra viết phương thức swap() riêng cho Array1D. Vì nếu ko thì người dùng phải swap 2 mảng với nhau bằng cách xài mảng tạm thứ 3, như vậy là tạo 1 mảng ví dụ 10 ngàn phần tử tạm thời. Chưa kể phải gán 3 bước nữa, như vậy là 30 ngàn phép gán. Trong khi swap con trỏ kiểu này chỉ cần 2 biến tạm thời và ko quá 10 phép gán. Vậy là viết được thêm phương thức swap nữa Có phương thức swap rồi thì trong operator= chỉ việc ghi 1 dòng this->swap(x); hoặc ko cần this: swap(x); là xong

void Array1D::swap(Array1D& other) { /*hoán đổi n và hoán đổi a*/ }

Array1D& Array1D::operator=(Array1D x) { swap(x); }