Thread: String Class

Originally Posted by vart

1. stringlength member may be not initialized or initialized not properly
2. temp is pointing to the not initialized buffer buf
3. strcpy(buf, temp); has no effect - you copying buffer over itself (maybe you think you copy the old buffer contents - but you effectively lost it making memory leak)
4. strcat(buf, str.buf); - you appending to not initialized buffer - behavior is undefined
5. delete []temp; - you deleting newly allocated buffer leaving your object in the dengerous state

do I even need temp?

Are there any memory leaks with this code? Does size need to be bigger?

Code:

String& String::operator +=(const String & str)
{
	int newsize = getLength() + str.getLength();
	stringlength = newsize;
	size =  newsize + 1;
	char *temp = buf;
	buf = new char[size];
	strcpy(buf, temp);
	strcpy(buf + getLength(), str.buf);
	return *this;
}

Originally Posted by vart

Yes. You do not delete the old buffer


getLength returns stringlength wich already points to newsize index...
So you writing outside the allocated buffer

I managed to change the code to look like this, but there are still problems with it:

Code:

String& String::operator +=(const String & str)
{
	int newsize = getLength() + str.getLength();
	size =  newsize + 1;
	char *temp = new char[size];
	assert(temp != 0);
	strcpy(temp, buf);
	delete []buf;
	buf = temp;
	strcat(buf + getLength(), str.buf);
	stringlength = newsize;
	return *this;
}

Originally Posted by vart

strcat(buf + getLength(), str.buf);
should be
strcat(buf, str.buf);

Now there is a break at strcat(buf, str.buf);

Originally Posted by vart

And you have exemined the variables at this point and found that...

I ran the debugger line by line and it seemed to be running fine then all of a sudden buf and temp went from ABCD to ABCDABCDABCDABCD and it never ends and a break pops up that says:
Unhandled exception at 0x102aece9 in String Lab.exe: 0xC0000005: Access violation writing location 0x00359000.

Does anybody see anything wrong with my +operators:

Code:

String operator +(const String& lhs, const String& rhs)
{
	String result(lhs);
	result += rhs;
	return result;
}

String operator +(char str, const String& rhs)
{
	String result;
	result = str;
	result += rhs;
	return result;
}

String operator +(const String& rhs, char str)
{
	String result(rhs);
	result += str;
	return result;
}

String operator +(const char* str, const String& rhs)
{
	String result;
	result = str;
	result += rhs;
	return result;
}

String operator +(const String& rhs, const char* str)
{
	String result(rhs);
	result += str;
	return result;
}

Have you fixed all of your constructors?

Originally Posted by Daved

Have you fixed all of your constructors?

I think they all work but Ill post my code below and if you see anything then please tell me.

Code:

//string.h

#ifndef String_H
#define String_H

#include <iostream>
#include "strdrv.h"
using namespace std;

class String
{
	friend String operator+(const String& lhs, const String& rhs);
	friend String operator+(const String& rhs, const char* str);
	friend String operator+(const char* str, const String& rhs);
	friend String operator+(const String& rhs, char str);
	friend String operator+(char str, const String& rhs);
	friend int operator==(const String& lhs, const String& rhs);
	friend int operator!=(const String& lhs, const String& rhs);
	friend int operator< (const String& lhs, const String& rhs);
	friend int operator<=(const String& lhs, const String& rhs);
	friend int operator> (const String& lhs, const String& rhs);
	friend int operator>=(const String& lhs, const String& rhs);
	friend char* operator+(const String &rhs, int element);
	friend char* operator+(int element, const String &rhs);
	friend ostream& operator<<(ostream&, const String&);

public:
	String();
	String(const char* str);
	String(char nme);
	String(int sizeofstring);
	String(const String &rhs);
	String(char letter, int repeat);
	~String();
	void setName(const char* aname);
	String& operator= (const String &rhs);
	String& operator= (const char *st);
	String& operator= (const char st);
	String& operator+=(const String & str);
	String operator+() const;
	char& operator[](int i);
	char& operator[](int i) const;
	String& operator++();
	String& operator--();
	String operator++(int x);
	String operator--(int x);
	int getLength()const;
	String substr(int start, int length);
	void print();
	
    class IndexOutOfBoundsException : public std::out_of_range
    {
    public:
		IndexOutOfBoundsException() : std::out_of_range("IndexOutOfBoundsException") { }
	};



private:
	int stringlength;
	int size;
	char * name;
	char *buf;

};

#endif 

// string.cpp
#define _CRT_SECURE_NO_DEPRECATE 1	
#include <iostream>
#include <assert.h>
#include "string.h"


using namespace std;

String::String()
{
	name = NULL;
	stringlength = 0;
	size = stringlength + 1;
	buf = new char[size];
	memset(buf, 0, size);
	assert(buf != 0);
	buf[0] = '\0';
}

String::String(const char *str)
{ 
	name = NULL;
	assert( str != 0);
	stringlength = strlen(str);
	size = stringlength + 1;
	buf = new char[size];
	memset(buf, 0, size);
	strcpy(buf, str);
}

String::String(char nme)
{
	name = NULL;
	stringlength = 1;
	size = stringlength + 1;
	buf = new char[size];
	memset(buf, 0, size);
	buf[0] = nme;
	buf[1] = '\0';
}

String::String(int sizeofstring)
{
	name = NULL;
	assert(sizeofstring >= 0);
	stringlength = sizeofstring;
	size = stringlength + 1;
	buf = new char[size];
	memset(buf, 0, size);
	assert(buf != 0);
	for(int i =0; i< size; i++)
		buf[i] = '\0';

}

String::String(char letter, int repeat) : name(0), stringlength(repeat)
{
	buf = new char[repeat+1];
    memset( buf, 0, repeat + 1);
	for(int i= 0; i< repeat; i++)
		buf[i] = letter;
}

String::String(const String &rhs)
{
	name = NULL;
	stringlength = rhs.stringlength;
	size = stringlength + 1;
	buf = new char[size];
	memset(buf, 0, size);
	assert(buf != 0);
	for (int i=0; i <= stringlength; i++)
		buf[i] = rhs.buf[i];
}

String::~String()
{
	delete []buf;

}

void String::setName(const char* aname)	
{
	name = new char[strlen(aname + 1)];		
	strcpy(name, aname);					
}

String& String::operator =(const String &rhs)
{
	if(this != &rhs)
	{
		delete []buf;
		stringlength = rhs.stringlength;
		buf = new char[stringlength + 1];
		memset(buf, 0, stringlength + 1);
		strcpy(buf, rhs.buf);
	}
	return *this;
}

String& String::operator =(const char *st)
{
	String temp(st);
	return operator =(temp);
}

String& String::operator =(char st)
{
	stringlength = 1;
	size = stringlength + 1;
	buf = new char[size];
	memset(buf, 0 , size);
	buf[0] = st;
	buf[1] = '\0';
	return *this;
}

char& String::operator [](int i) const 
{
	if(i >= 0 && i < stringlength)
		throw IndexOutOfBoundsException();
	return buf[i];
}

char& String::operator [](int i)
{
	if(i >= 0 && i < stringlength)
		throw IndexOutOfBoundsException();
	return buf[i];
}

String& String::operator +=(const String & str)
{
	int newsize = getLength() + str.getLength();
	size = newsize + 1;
	char *temp = new char[size];
	memset(temp, 0, size);
	strcpy(temp, buf);
	strcat(temp, str.buf);
	delete []buf;
	buf = temp;
	stringlength = newsize;
	return *this;
}

int String::getLength()const
{
	return stringlength;
}


String operator +(const String& lhs, const String& rhs)
{
	String result(lhs);
	result += rhs;
	return result;
}

String operator +(char str, const String& rhs)
{
	String result(str);
	result += rhs;
	return result;
}

String operator +(const String& rhs, char str)
{
	String result(rhs);
	result += str;
	return result;
}

String operator +(const char* str, const String& rhs)
{
	String result(str);
	result += rhs;
	return result;
}

String operator +(const String& rhs, const char* str)
{
	String result(rhs);
	result += str;
	return result;
}



String String::substr(int start, int length)
{
	if(start >= 0 && start < stringlength)
		cout<< "Start of substring not in string bounds" << endl;
	if(length >= 0)
		cout<< "Error: length of substring is negative" << endl;
	if((start + length) > stringlength)
		length = stringlength - start;
	char* stringArray = new char[length + 1];
	memset(stringArray, 0, length +1);
	int i;
	for(i = start; i < (start + length) ; i++)
		stringArray[i-start] = (*this)[i];
	stringArray[i - start] = '\0';
	String temp(stringArray);
	delete []stringArray;
	return temp;
}



int operator==(const String& lhs, const String& rhs)
{
	return strcmp(lhs.buf, rhs.buf)==0;
}

int operator!= (const String& lhs, const String& rhs)
{
	return !(lhs.buf == rhs.buf);
}

int operator< (const String& lhs, const String& rhs)
{
	return strcmp(lhs.buf, rhs.buf) <0;
}

int operator<= (const String& lhs, const String&rhs)
{
	return strcmp(lhs.buf, rhs.buf) <=0;
}

int operator> (const String& lhs, const String& rhs)
{
	return strcmp(lhs.buf, rhs.buf) >0;
}

int operator>= (const String& lhs, const String& rhs)
{
	return strcmp(lhs.buf, rhs.buf) >=0;
}

ostream& operator<< (ostream& output, const String& stringoutput)
{
	output<< stringoutput.buf;
	return output;
}


String& String::operator ++()
{
	for(int i = 0; i<stringlength; i++)
		++buf[i];
	return *this;

}

String String::operator ++(int x)
{
	++buf[x];
	return *this;

}

String& String::operator --()
{
	for(int i = 0; i<stringlength; i++)
		--buf[i];
	return *this;
}

String String::operator --(int x)
{
	--buf[x];
	return *this;
}

void String::print()
{
	for(int i= 0; i<stringlength; i++)
		printf("&#37;c", buf[i]);
}

>> String::String(char letter, int repeat) : name(0), stringlength(repeat)
That constructor isn't quite right, but from the looks of it you added it recently and with a different style than your other ones.

The other constructors have solved the initialization problem I mentioned before, except now one of your operator= does the same bad thing of not setting all values to valid states.

Look at how your operator+= sets stringlength at the end of the function. It's not quite right.

Your operator=(char) has a memory leak. Remember that you need to clean up existing memory inside operator=. Also check your setName function for the same problem.

I assume you haven't used your operator[] functions yet. You'll see the problem when you do.


Do you have test code? You should write a small test app that tests one function at a time. Start with a string, run one of the functions, and check the values inside the string to make sure they are accurate. do this for every function separately, and then they should work better when you combine them.