Making my own String class:

[xxhashxx]

I am writing my own string class library, that contains basic functions of the string class, along with overloading the +,==,>,<,<< and >> operators. I have defined the "MyString" class header file as follows.

# include <iostream>
using namespace std;
class MyString
{
friend bool operator == (const MyString & S1, const MyString & S2);
friend bool operator < (const MyString & S1, const MyString & S2);
friend bool operator > (const MyString & S1, const MyString & S2);
friend MyString operator +(MyString &S1 , MyString & S2);
	
	
public:
	MyString();//
	MyString(char *STR);//
	MyString(MyString & STR);//
	virtual ~MyString();
    
   	MyString substr(int index,int length);
	int find(const MyString& str, int index = 0);
	char& at(int loc);
	void swap(MyString& from);
	
	int length();//
	bool empty();// check for empty string
		
	char* get_string(){return DATA;}
	int get_length(){return str_length;}
	void set_data(char * input, MyString &S);
private:
	int str_length;
	char *DATA;
};
ostream& operator <<(ostream& OS, MyString &S);
istream& operator >>(istream& IS, MyString &S);

My constructors work, and i can create a default object and cin the the string into the object and later cout the string.

my problem arises when i am overloading the + operator.

the definition for the + operator:

MyString operator +(MyString& S1, MyString &S2)
{
int size = S1.str_length + S2.str_length + 1;
char* temp = new char[size];

memcpy(temp,S1.DATA,S1.str_length);

strcat(temp,S2.DATA);
cout << temp << " " << strlen(temp)<< endl;

MyString K;
K.set_data(temp,K);
delete[]temp;
return K;
}

void MyString::set_data(char *input, MyString &S)
{
str_length = strlen(input)+1;
DATA = new char[str_length];
memcpy(DATA,input,str_length);
}

When i say S1("sam") and S2("jhon"), and later on in the code i say S1 + S2, i expect an output of "samjhon",of size 7 excluding the "\0", but i get a string of size 37 outputting a string of gibberish.In the function i place a cout statement for the temp variable, and it contains the right string of the right size. So how is the temp not getting copied over to the obj k.

the following is default consrutor and the <<,>> operators:

MyString::MyString()
{
str_length = 1;
DATA = new char[str_length];
memcpy(DATA,"",str_length);
}


ostream& operator << (ostream& OS, MyString & S)
{
OS << S.get_string();
return OS;
}

istream& operator >> (istream& IS, MyString & S)
{
char* input = new char[100];
IS.getline(input,100);
S.set_data(input,S);
delete[] input;
return IS;
}

Please suggest any ideas, to rectify this.
thanks

[kbw]

First a few questions.

1. Have you considered the use of const?
MyString(MyString & STR); is very different from
MyString(const const MyString & STR);
The compiler will generate the second if it's not found, and in your case, it's the wrong thing.

stream& operator << (ostream& OS, MyString & S) should be
stream& operator << (ostream& OS, MyString & S) *EDIT*

2. Why are you using memcpy when working with strings? You need to be consistent.

3. Memory sizes are of type size_t, not specifically int. You ought to be warned by your compiler about this.

4. Why is your destructor virtual?

5. Why is set_data not private? Plus you allocate memory, pass it in to set_data, which copies the block and then it's deleted by the caller. Why doesn't it just take ownership of the block and use it? And why does it take a string as a parameter?


You haven't posted the code for your destructor, I take it it deletes DATA.

The problem you've reported is caused by point 1. The compiler call its copy constructor (not yours) and so the memory its using is deleted during the copy.

[xxhashxx]

thanks kbw for your reply, following your suggestions, i rewrote the program from scratch and got better results:

class:

class MyString
{

public:
	MyString(void);//default sets string to NULL
	MyString(const char* input);//Takes in a string and creates an object
	MyString(const MyString & S);//Copy constructor
	~MyString(void);//Destructor deletes the nstring at the end
    
    
	int length();//displays length of string excluding null
	char& at(int loc);// returns a chracter at specified loc-1
	bool empty();// if string is empty returns true else it returns false
    void swap(MyString& from);// swaps the contents of two strings
	int find (const MyString& S, int index =0);
	MyString substr(int index, int length);// returns an object which is the substring of the object that calls it


friend ostream& operator << (ostream & OS,const MyString &S);// out stream displays the string
friend istream& operator >> (istream & IS,MyString &S);//instream takes in a string and creates and object
MyString& operator = (const MyString& S);//if A and B are object B is "neil", the A =B therefore A is "neil"
friend  bool operator ==(const MyString& S1, const MyString & S2);// checks if both strings are equal
friend MyString operator + (const MyString& S1, const MyString & S2);//still buggy

private:
	char* nstring;
	short nlength;
};

I made changes to my class.

The following is the implementation of the functions and constructors:

# include "MyString.h"
# include <iostream>
# include <cstring>

using namespace std;

/*temp = is a temporary variable*/

MyString::MyString()
{
nlength = 1;
nstring = new char[nlength];
nstring[nlength-1] = '\0';
}

MyString::MyString(const char *input)
{
nlength = strlen(input)+1 ;
nstring = new char[nlength];
for(int i = 0; i < (nlength - 1); i++)
{
nstring[i] = input[i];
}
nstring[(nlength-1)] = NULL;
}

MyString::MyString(const MyString& S)
{
nlength = S.nlength;
nstring = new char[nlength];
for(int i = 0; i < (nlength - 1); i++)
{
	nstring[i] = S.nstring[i];
}

if(nstring[nlength - 1] != '\0'){nstring[nlength -1] = '\0';}
}

MyString::~MyString()
{
delete[] nstring;
}


int MyString::length()
{
int i = 0;
while(nstring[i] != '\0')
{i++;}
return i;
}

char& MyString::at(int loc)
{
return nstring[loc - 1];
}

bool MyString::empty()
{
	if(nstring[0] == NULL && nlength == 1){return true;}
	else {return false;}
}

void MyString::swap(MyString &from)
{
short lena = nlength;short lenb = from.nlength;
char *temp1 = new char[lena];
char *temp2 = new char[lenb];

for(int i = 0; i < (lena- 1); i++)
{
	temp1[i] = nstring[i];
}
if(temp1[lena - 1] != '\0'){temp1[lena -1] = '\0';}

for(int j = 0; j < (lenb- 1); j++)
{
	temp2[j] = from.nstring[j];
}
if(temp2[lenb - 1] != '\0'){temp2[lenb -1] = '\0';}

delete[] nstring;delete[]from.nstring;

nstring = new char[lenb];
nlength = lenb;
from.nstring = new char[lena];
from.nlength = lena;

for(int k = 0; k < (lenb - 1); k++)
{
nstring[k] = temp2[k];
}
if(nstring[lenb -1] != '\0'){nstring[lenb - 1] = '\0';}

for(int l = 0; l < (lena - 1); l++)
{
from.nstring[l] = temp1[l];
}
if(from.nstring[lena -1] != '\0'){from.nstring[lena-1] = '\0';}

delete[]temp1; delete[]temp2;
}


MyString MyString::substr(int index, int length)
{
short size = length+1;
char* temp = new char[size];
int j = 0;
for(int i = index, j = 0; i < length; i++, j++)
{
temp[j] = nstring[i];
}
if(temp[size -1 ] != '\0'){temp[size -1] = '\0';}


MyString RET;
RET.nlength = size;
delete[]RET.nstring;
RET.nstring = new char[size];
for(short m = 0; m < (size - 1);m++)
{
	RET.nstring[m] = temp[m];
}

if(RET.nstring[size-1] != '\0'){RET.nstring[size-1] = '\0';}
return RET; 
delete[]temp;
}



bool operator == (const MyString & S1, const MyString & S2)
{
	if(S1.nlength == S2.nlength)
	{
		short counter1 = S1.nlength;
		int counter2 = 0;
		int i = 0;
		while(i != counter1)
		{
			if(S1.nstring[i] != S2.nstring[i]){counter2++;}
			i++;
		}
		cout << i << " " << counter2 << endl;
		if(counter2 != 0) {return false;}
		else{return true;}		
	}
	else {return false;}

}

ostream& operator <<(ostream & OS, const MyString& S)
{
OS << S.nstring;
return OS;
}

istream& operator >>(istream & IS,MyString& S)
{
char* input = new char[100];
IS.getline(input,100);

int i = 0;
while(input[i] != '\0')
{i++;}
S.nlength = i+1;
delete[]S.nstring;
S.nstring = new char[(i+1)];
for(int j = 0; j < (i) ; j++)
{
	S.nstring[j] = input[j];
}
S.nstring[i] = '\0';
delete[]input;
return IS;
}

MyString& MyString::operator =(const MyString &S)
{
nlength = S.nlength;
delete[] nstring;
nstring = new char[nlength];
for(int i = 0 ; i < (nlength -1);i++)
{
nstring[i] = S.nstring[i];
}
nstring[(nlength -1)] = '\0';
return *this;
}

MyString operator + (const MyString& S1, const MyString & S2)
{
int size = S1.nlength + S2.nlength +1;
char * temp = new char[size];
for(int i = 0; i < (S1.nlength -1);i++)
{
temp[i] = S1.nstring[i];
}

int j = 0;
for(short k = S1.nlength , j=0; k < (size-1) ; k++, j++)
{
temp[k] = S2.nstring[j];
}
if(temp[size -1 ] != '\0'){temp[size -1] = '\0';}

MyString S3;
S3.nlength = size;
delete[] S3.nstring;
S3.nstring = new char [size];
for(int z = 0; z < (size -1); z++)
{
S3.nstring[z] = temp[z];
}
if(S3.nstring[size-1] != '\0'){S3.nstring = '\0';}
return S3;
delete[] temp;
}

everything works well for example:

MyString A("max"),B,C;
cin >> B;//cin" jhon"
A.swap(B);
C = A.substr(0,3);
cout << C;

C = "jho"

The problem arises with the overloaded "+" operator, any time i say A+B etc,
i get a "Unhandled Exception error, Access Violation reading loaction". It then show me that some thing went wrong when values passed my copy constructor.

Does any one have any idea, whats happening?
Thanks in advance.

[Duthomhas]

@kbw
point 1: I think you meant to write:
stream& operator << (ostream& OS, MyString & S) should be
stream& operator << (ostream& OS, const MyString & S)

@xxhashxx
You need to use a better coding style and pay close attention to what you are doing. It is tripping you up to be so tricky.

For example, it is an INVARIANT that your string's 'nlength' member is exactly (the number of characters in the string plus one), and your 'nstring' member contains exactly 'nlength' characters, and the very last character must be '\0' (not NULL, by the way, which is something different).

Hence, your constructors could be written much more succinctly:

MyString::MyString()
{
  // A new, empty string contains zero characters plus a terminating zero.
  nlength = 1;
  nstring = new char[1];
  nstring[0] = '\0';
}

MyString::MyString(const char *input)
{
  // A new, copy of a C-string contains exactly the same number of characters in
  // the C-string plus a terminating zero.
  nlength = strlen( input ) + 1;
  nstring = new char[ nlength ];
  for (int i = 0; i < nlength; i++)
    nstring[ i ] = input[ i ];
}

For good measure, that second one could have been written as:

MyString::MyString(const char *input)
{
  // A new, copy of a C-string contains exactly the same number of characters in
  // the C-string plus a terminating zero.
  nlength = strlen( input ) + 1;
  nstring = new char[ nlength ];
  strcpy( nstring, input );
}

The copy constructor should be just as plain:

MyString::MyString(const MyString& S)
{
  nlength = S.nlength;  // we know this is correct!
  nstring = new char[ nlength ];
  strcpy( nstring, S.nstring );
}

Now consider your [edit] destructor length function [/edit], which does essentially what strlen() does -- but in any case you don't need either a loop or strlen(), because you know that nlength is (the number of characters in the plus one)! You try:

int MyString::length()
{
  return /* return something simple: no loops and no strlen() allowed here! */
}

Your operator+() function is doing the same kind of thing. You've got so many things happening in there you are getting lost. Hint: use strcpy() and strcat(). No loops necessary. It will take you all of seven lines of code:
1 create your result string (which you named "S3")
2 delete[] the old 'nstring'
3 calculate result string's new length
4 allocate the new nstring with new
5 copy S1's data to the new nstring
6 concatenate S2's data to the new nstring
7 return the new string

Also watch out for the order in which things happen. Notice how line 217 returns S3, but line 218 delete[]s the temporary data. Of course, line 218 can never happen, because you returned before that.

I'm running out of pep, but it would be a really good idea to fix your operator==() also.

Hope this helps.

[xxhashxx]

@ duoas

i was going to use the cstring library functions i.e. strcpy() strcmp(), to define my member functions. But my professor asked us to make our own functions and limit our use of the cstring functions. Hence why i ended up using som many loops.

i have some questions:

Now consider your destructor, which does essentially what strlen() does

what exacty do you mean here?
And should i make my destructor virtual since i am using dynamic memory and pointers.

but in any case you don't need either a loop or strlen(), because you know that nlength is (the number of characters in the plus one)! You try:

yeah i realised that i should just :
return nlength-1;
instead of getting into loops.

for(int z = 0; z < (size -1); z++)
{
S3.nstring[z] = temp[z];
}
if(S3.nstring[size-1] != '\0'){S3.nstring = '\0';}
return S3;
delete[] temp;

yeah that wrong also, thanks for pointing that out , so i should delete[] temp before return S3.

i know that "\0" refers to an empty string, and '\0' refers the termination point of a string , so what is NULL exactly?

Just wanted to clarify , if i input a name as so
MyString A("bob"), am i passing "bob" to the function or "bob\0" ,

if it is the first case then nlength will equal to 3+1 = 4
so if i am going to enter values into the array , i will start at index 0.

0 = b
1 = o
2 = b
3 = \0

which is why i made it nlength-1.

my ==() seems to be okay for the following:

MyString A("seal"),B("seal"),C("bob"),D("bobe");
bool case1 = (A == B);
bool case2 = (A == C);
bool case3 = (A == D);
cout << case1 << endl;
cout << case2 << endl;
cout << case3 << endl;

1
0
0

i also want to know

if i say this:

MyString A,B,C;
cin >> A;
cin >> B;
C=A+B;// what exactly happens here? 

when i say this A+B, the +() is caled and computes the temp value and passes it to an object and returns it, i defined a =() in my code and tested it also and that works.

C = (A+B) when the value is returned does the compiler call the copy constructor, since the debugger always points there, or does it invoke the =()

again thanks for you help

[Duthomhas]

Oops... didn't mean to write "destructor". I fixed my post above.

A "string" is just an array of char stored somewhere in memory, where the last character is the "nul" character.

Name          Type      Value     Meaning
NULL          pointer   0         An invalid pointer value -- it cannot be dereferenced.
nul           char      '\0'      The ASCII NUL character
empty string  char[]    ""        A string containing no characters other than the terminating nul.

In C and C++, all arrays are handled via pointers. First, you must know something about memory spaces. What follows is simplified tremendously.

Memory is nothing but a numbered array of cells. The operating system controls which of those cells your program uses. Within your program, there are cells that store the machine code that tells the computer what to do. This is traditionally called the "text" segment. There are also cells that store data literals that you write into your program -- like "Hello world!". This is traditionally called the "data" segment. There are cells that are used to store temporary data -- variables you create inside functions and the like. This is called the "stack". And finally, there are cells that you access with new and delete. This is called the "heap".

In both the following cases, the double-quoted stuff on the right is an array of char stored somewhere in your program's data segment. The first example assigns the address of the very first character in the array to the pointer variable greeting. The second example makes a copy of the string "May the road rise to meet you!" (which is in the data segment) into a variable created on the stack.

const char* greeting = "Hello, handsome!";
char farewell[] = "May the road rise to meet you!";

In both cases, you now have access to two pointers:
- greeting, which points to the 'H' in the string "Hello, handsome" in the data segment;
- farewell, which points to the 'M' in the string "May the road rise to meet you!" on the stack (notice now that there are two copies of the string "May the road rise to meet you!" in your program -- one in the data segment and one on the stack).

Hopefully that will help when you think about strings now...

The reason you don't need to explicitly access nstring[ nlength - 1 ] is that, while correct, it is always going to be zero.

For example:

MyString::MyString(const char *input)
{
  // Count how many characters we need to store the input string
  for (nlength = 0; input[ nlength ] != 0; nlength++);
  // Also count the terminating nul character
  nlength += 1;
  // Get space from the heap for the string
  nstring = new char[ nlength ];
  // Copy all the characters, including the terminating nul, into our heap space
  for (int i = 0; i < nlength; i++)
    nstring[ i ] = input[ i ];
}

If input points to the 'B' in the string "Bob" (which is the same as the array { 'B', 'o', 'b', '\0' }, then line 4 will count three non-nul characters. Line 6 will add one to that, so that nlength is four -- the exact number of characters needed to store the string and its nul-terminator. Next we need somewhere to put the copy, so we get a chunk of cells from the heap on line 8. Finally, lines 10 and 11 copy the input array to our chunk of cells on the heap.

Hmm, one other thing: nstring[ i ] is just a fancy shortcut for *(nstring + i).

Hope this helps.

[xxhashxx]

yeah i managed to fix the overloaded + operator

MyString operator + (const MyString& S1, const MyString & S2)
{
int size = (S1.nlength -1) + (S2.nlength -1) +1;
char * temp = new char[size];
for(int i = 0; i < (S1.nlength -1);i++)
{
temp[i] = S1.nstring[i];
}

int j = 0;
for(short k = S1.nlength - 1 , j=0; k < (size-1) ; k++, j++)
{temp[k] = S2.nstring[j];}

if(temp[size -1 ] != '\0'){temp[size -1] = '\0';}

MyString S3(temp);
delete[] temp;
return S3;
} 

I made use of the copy constructor and passed the temp variable to it, deleted temp first and then returneed the object.

Secondly my intital calculation of size was wrong;
i had said size = S1.nlength + S2.nlength + 1;
this also includes the space for the terminator '\0'

the rest were some errors in the for loops which i rectified: so the following

MyString A, B,C ;
cin >> A;
cin >> B;
C = A + " " + B;
MyString D(A + " " + B);
cout << C << C.length() << endl;
cout << D << D.length() << endl;

works well, and returns the right size of the string.


i left the destructor as

virtual ~ MyString(){delete[] nstring;}

the rest i made some few changes to,

thanks to Duoas and kbw