I've inserted a few remarks below at distinct places. Please note that I am not an experienced C++ programmer.
--- In [email protected], "tery_bboy" <[EMAIL PROTECTED]> wrote: > > I wrote the following program to do string manipulation > using overloaded operators! See the code first then I > explain the error! (It might be a bit too long) > > #include <iostream.h> > #include <conio.h> To me this looks as if you're working with the ancient Turbo-C++ compiler. Get rid of this compiler and install something more modern. For programming under Windows, I suggest you use the C++ environment from Microsoft (see Links section of this Yahoo! group, it's free and according to the experts pretty good). Turbo-C++ does not understand regular valid C++ code adhering to the latest standards, and its technical basis is outdated to the core. > #include <stdio.h> > #include <string.h> > > class string > { > private: > char * str; You do not always make sure that "str" is initialised correctly. > int len; > public: > string(); > string(char *); > friend string operator + (const string &, const string &); > friend string operator - (const string &, const string &); > void display(void) > > { > cout<<str<<endl<<endl; > } > }; > > string :: string () > { > len = 0; > str = ""; This is a typical error: you have "str" point to some possibly transient memory area. This means: it is by no means guaranteed that the empty string you're addressing here is available to "str" for the whole duration of your program. Please make yourself familiar with the term "lifetime of objects". Without wanting to go into technical details here (which may vary between different compilers anyway), please just trust me that this is bad programming style as the memory area containing the empty string may not exist anymore after this constructor has finished its work, meaning that "str" could point to any possible memory content. > } > > string :: string (char * a) > { > len = strlen (a); > str = new char [len + 1]; > strcpy(str,a); > } In principle no bad idea, but you missed out the important check whether "a" is NULL or not. This doesn't cause any trouble in your sample source code, but in general this is a serious flaw. > > string operator + (const string & T, const string & S) > { > string temp; > temp.len = T.len + S.len; > temp.str = new char [temp.len + 1]; > strcpy(temp.str, T.str); > strcat(temp.str, S.str); > return (temp); > } Here you've missed out checking whether any of the strings are not set at all (meaning: are NULL). > string operator - (const string & T, const string & S) > { > string temp; > char * substr; > substr = strstr(T.str,S.str); > if (substr == NULL) > { > cout<<"Substring not found! "; > return(temp); > } > > else if (substr == T.str) > { > temp.len = T.len-S.len; > temp.str = new char [temp.len + 1]; > strcpy(temp.str,T.str); > strnset(temp.str,' ',S.len); > strrev(temp.str); > temp.str[temp.len] = '\0'; > strrev(temp.str); > } > else > { > temp.len = strlen(T.str); > temp.str = new char [temp.len + 1]; > strcpy(temp.str,T.str); //HERE IS WHERE IT ALL HAPPENS > strrev(temp.str); > strnset(temp.str ,' ',S.len); > strrev(temp.str); > } > return (temp); The big point is: what exactly is "<string> - <string>" supposed to return? I don't understand what this code should do. > } > > void main() > { > clrscr(); > char * tmp_str = new char [30]; > char * tmp_str2 = new char [30]; > cout<<"Enter string 1: "; > gets(tmp_str); > cout<<"Enter string 2: "; > gets(tmp_str2); gets() is as flaky and dangerous to use as can be. Please, never ever use this function (except if you have checked _in advance_ that the string is short enough to fit into the buffer), but get into the habit of using fgets() as long as you're dealing with "char*" strings. > string S1(tmp_str), S2(tmp_str2), S3, S4, S5; > > delete []tmp_str; > delete []tmp_str2; > > S3 = S1 + S2; > S4 = S3 - S1; > S5 = S3 - S2; > > cout<<endl<<endl; > cout<<"S1: ";S1.display(); > cout<<"S2: ";S2.display(); > cout<<"S3 = S1 + S2: ";S3.display(); > cout<<"S4 = S3 - S1: ";S4.display(); > cout<<"S5 = S3 - S2: ";S5.display(); > > getch(); > } > > If i give the input: > String 1: Hello(no space) > String 2: World(one space before world) > output is: > S1: Hello > S2: World > S3: Hello World > S4: World > S5: Hello > > Perfect. > > If I give this input: > String 1: Hello (space) > String 2: World (Space or no space before world) > output is: > S1: Hello > S2: World > S3: Nothing is printed (which should this be affected?) > S4: World > S5: Nothing is printed <anip> That's all I can say right now. I hope some C++ experts will comment on those issues where I can't. Regards, Nico
