🚀 go-pugleaf

In comp.lang.c++ SKAS <f93c0532@yahoo.com> wrote:
> I've written a class for string datatype. The program runs fine but
> crashes in the end.

Your constructor is broken, as are a few more things; see below.

BTW: you seem to save whitespaces, like writing cout<<"foo"<<endl; I'd
advise you to make more liberal use of those, like cout << "foo << endl;
It is more readable, IMHO.

> I'm using turbo c++ 3.0.

Okay, since TC3 is old an compiler, I won't whine about using old header
names. Remember though conio.h is non-standard, and won't work in other
than Borland's products.

>   string(char a[])
>   {
>    strcpy(head,a);
>   }

Head is a char pointer, which doesn't necessilary point anywhere. In
your code, you say

string s1("the 1st and most IMPORTANT thing");

which calls for appropriate constructor, i.e. such that takes a char
array as parameter. The constructor never allocates the memory for the
new string, and that's a mistake. Let's remake the constructor like
this:

string(char *a){
 try{
  a=new char[strlen(a)+1]; // allocate new array and add 1 for \0 terminator
 }
 catch(bad_alloc &b){ // check if the allocation worked OK.
  std::cout << b.what();
  exit(1);
 }

 strcpy(head, a);
 // you might even want to check the head has equal length with a.
}

>   ~string()
>   {
>      delete head;
>   }

Invalid destructor, head is allocated with new char[], thus you must
delete[] it:

~string(){
 delete []head;
}

> void string::print()
> {
>  int index=0;
>  while (*(head+index))
>  {
>   cout<<*(head+index);
>   index++;
>  }
> }

cout is buffered, so it won't necessilary print anything onto screen.
You can force it by printing a std::endl, or std::flush;

Add cout << flush or cout << endl after the while loop. (Since TC3,
IIRC, doesn't support namespaces, you won't need the std:: -prefix.
Should be used in standard C++ though...)


> char string::atindex(int index)
> {
>     return *(head+index);
> }

Does the user know the 1st character lives in location 0? This is
self-evident for any programmer, but the rest of us get confused as they
ask the 1st char and get H instead of T.

> void string::upper()
> {
>  int index=0;
>  while (*(head+index))
>  {
>   if ((*(head+index)>=97) && (*(head+index)<=122))
>    *(head+index)-=32;
>   index++;
>  }
> }

I find this style ugly. You can easily find the length of your head
array by strlen() function call, and use it as an upper limit for loop
instead of string pointers.

By the way... you are modifying the original string. After a case change
the original is lost. Consider

Original string (1st char uppercase, rest lowercase)
ORIGINAL STRING (all uppercase)
original string (all lowercase)

Maybe you want to store the original string and modify a temporary one
instead.

> void main(void)

Main should return an int.

>  gets(sub);

Never, ever use gets! Gets is EVIL. The problem is, gets doesn't check
whether your input is longer than the buffer you assign to it. If you
input more than what will fit into the buffer, you might crash the
application. Even worse, this is excactly the same idea behind so many
IIS worms are cracking into web servers.

Use
std::cin.ignore(1, '\n'); // to get rid of last endline entered
std::cin.getline(sub, 29, '\n'); // read up to 29 chrs to the 30 chr buffer


-P

-- 
Thou shalt not follow the Null Pointer,
for at its end Madness and Chaos lie.