Heap Corruption

[Athar]

Also, const CString string was probably supposed to be const CString& string.
Without it, an unnecessary copy of the parameter will be made every time you call one of these functions.

[ascii]

@Athar, I was just testing out my copy constructor. I'll make them all references now ;)

Still gives me a stupid Debug Error. Here's the whole program.

#include<iostream>
#include<cstring>

class CString
{
public:
	//Constructors
	CString(const char* string)
	{
		str = new char[strlen(string)+1];
		strcpy(str, string);
		length = strlen(string+1);
	}
	CString(const CString& string)
	{
		str = new char[string.length+1];
		strcpy(str, string.str);
		length = string.length+1;
	}

	~CString() 
	{ delete[] str; }

	CString& operator=(const CString string);
	CString operator+(const CString& string) const;
	CString& operator+=(const CString& string);
	void print() const;

private:
	char* str;
	size_t length;
};

CString& CString::operator=(const CString& string)
{
	if(this == &string)		// return if the two are the same
		return *this;

	delete[] str;
	this->str = new char[string.length+1];
	strcpy(this->str, string.str);
	this->length = string.length;
	return *this;
}

CString CString::operator+(const CString& string) const
{
	char* temp = new char[this->length];
	strcat(temp, string.str);
	CString concatenated(temp);
	delete[] temp;
	return concatenated;
}

CString& CString::operator+=(const CString& string)
{
	this->length += string.length;
	char* temp = new char[this->length+1];
	strcat(temp, this->str);
	strcat(temp, string.str);
	delete[] this->str;
	this->str = temp;
	return *this;
}

void CString::print()   
{
	std::cout << str << std::endl; 
	return;
}

int main(void)
{
	char str[] = "Hello, world!";
	char* pstr = str;
	CString str1(pstr);
	CString str2 = str1;
	str1 += str2;
	str2.print();

	pstr = NULL;
	return 0;
}

As for overusing this->, just a bad habit I suppose :/

[Athar]

Doesn't your debugger show you where the error occurs?
The first error points to the strcpy in line 17. This is likely caused by lines 12 and 18, which should be
length = strlen(string); and
length = string.length;, respectively.

The second error points to the strcat in line 59. This is because you use the newly allocated array uninitialized.
temp[0]='\0'; will make it an empty C string.

[ascii]

Thank you Athar! I wasn't expecting the errors to be in the Constructor, good call! The + operator was much easier, so now the whole thing works :-)

#include<iostream>
#include<cstring>

class CString
{
public:
	//Constructors
	CString(const char* string)
	{
		str = new char[strlen(string)+1];
		strcpy(str, string);
		length = strlen(string);
	}
	CString(const CString& string)
	{
		str = new char[string.length+1];
		strcpy(str, string.str);
		length = string.length;
	}

	~CString() 
	{ delete[] str; }

	CString& operator=(const CString& string);
	CString operator+(const CString& string) const;
	CString& operator+=(const CString& string);
	void print() const;

private:
	char* str;
	size_t length;
};

CString& CString::operator=(const CString& string)
{
	if(this == &string)		// return if the two are the same
		return *this;

	delete[] str;
	this->str = new char[string.length+1];
	strcpy(this->str, string.str);
	this->length = string.length;
	return *this;
}

CString CString::operator+(const CString& string) const
{
	const int len = this->length + string.length;
	char* temp = new char[len];
	temp[0] = '\0';
	strcat(temp, this->str);
	strcat(temp, string.str);
	CString concatenated(temp);
	return concatenated;
}

CString& CString::operator+=(const CString& string)
{
	this->length += string.length;
	char* temp = new char[this->length+1];
	temp[0] = '\0';
	strcat(temp, this->str);
	strcat(temp, string.str);
	
	delete[] this->str;
	this->str = temp;
	return *this;
}

void CString::print() const   
{
	std::cout << str << std::endl; 
	return;
}

int main(void)
{
	char string1[] = "Hello, world!";
	char string2[] = "Goodbye, world!";
	char* pstr1 = string1;
	char* pstr2 = string2;

	CString str1(pstr1);
	str1.print();
	CString str2 = str1;
	str2.print();
	str1 += str2;
	str1.print();

	CString str3(pstr2);
	str1 = str2 + str3;
	str1.print();

	pstr1 = NULL;
	pstr2 = NULL;
	return 0;
}

EDIT: Woah, I just had an epiphany. I could've made the += operator much simpler by writing it like this:

CString& CString::operator+=(const CString& string)
{
	*this = *this + string;
	return *this;
}

[Athar]

Oh, while we're at operator+: it leaks memory because it doesn't delete the temp array and it allocates one character less than necessary.

Consider this:

CString CString::operator+(const CString& string) const
{
	CString concatenated(*this);
	concatenated+=string;
	return concatenated;
}

or short:

CString CString::operator+(const CString& string) const
{
	return CString(*this)+=string;
}

Edit: I see you had a similar idea, but one should generally implement operator+ using operator+=, not the other way round ;)

[ascii]

Alright thank you, I switched those two around :)